-
Notifications
You must be signed in to change notification settings - Fork 19
[WIP] Reexport API, add customization options + custom text + image loading #29
Conversation
…d ButtonBuilder API
So, I understand the temptation to deal with #21 and #22 at the same time but I really would prefer if any attempt to work on #21 was put off for the time being, I have some ideas about #21 that I think will require a far more substantial refactoring, I'll have more to say about this next week. Also fixing of re-exports should also be done in a separate PR. For font handling specifically, I think specifying TextState and by extension WidgetBuilder APIs that specify fonts, should only be concerned with font names, sizes and other font parameters rather than passing around So for instance, in this example:
Also I'm not sure why you removed the distinction between a font and a font instance, a font just holds the data for a loaded font, whereas a font instance caches data about a font at a specific size, there should be a difference. |
My reasoning for that was: If you have an Later on I want to be able to invoke garbage-collection on the fonts and / or safely delete them - i.e. the user should be able to say "remove all fonts that aren't needed". This is important for me because I'm trying to build a file explorer where you could go into a directory with lots and lots of fonts. If I'd just add fonts, I'd be out of memory very soon. If you have an Maybe an Regarding the The problem with remaking the Fonts is simply that it "bubbles up" - if you redo the font to be explicitly required, now the TextBuilder wants to know where to get the font from. If you redo the I'll do #23 in a seperate PR, though, you're right about that. Again, this is why I'm making the PR early, to get feedback. For #21, I'm curious to see what your idea is. My idea (with this PR) was to make with this is to delay the creation of a |
There will always be cases where you will want to or have to specify the font by name, either to make widget definitions that are reusable without bundling fonts, or when eventually you can serialize style/ui descriptions, in json or whatever. I'd rather not have separate or "weaker" APIs for those use cases since they are actually more important. Aside from the simplicity of having one way of specifying a font, it also gives motivation to improve the usability of this use case, by making the rules for it simple and well documented. To clarify, I'm saying there should be separate multiple ways of loading or registering a font, but only one of selecting a font.
What Then you can either just have it delete fonts as soon as nothing is using it to draw, or at least know when nothing is using it to draw if you want to manually trigger a font cleanup. |
Oh sorry I think I said #21 when I meant #22 before, but I think that's what you're talking about too. There isn't really a concrete plan here for styling, just some loose ideas, I'll write them up in that issue. actually the changes here don't seem like a big deal, mostly I'd just prefer it be in a separate PR. For the re-exports actually I implemented that myself as part of some other clean up, so I'll merge that soon. |
While I haven't forgotten about this PR, I have a lot of work to do. @christolliday it might be better to take the API that I proposed for the font and image loading and the improvements for the I have built my own ad-hoc layout framework which doesn't work very well, but for the stuff I need to do right now, it's OK. I'm sorry that I can't work on this a lot. I wish you good luck and I'll definitely stay in touch with the project - to get a pure-Rust drawing / UI solution. Thank you for doing this. |
@fschutt no problem, I can take care of it, have a few things that I'll probably do first but I'll get around to it eventually ;) thanks for the thought anyway and good luck with your stuff! |
This PR fixes #22, #23 and provides an alternative for font- and image loading (from #21).
This doesn't compile yet and I probably have to rebase the changes for a "proper" documentation, but since it's a bigger task, I wanted to be transparent with what I'm doing.
Also to avoid spamming #21 too much. Comments on the PR are welcome, even though it isn't done yet. However, I've seen that #21 and #22 are highly interleaved - you can't fix the one without fixing the other.
The goal is to defer the creation of a
WidgetBuilder
until an item gets actually used. This enables users to override styling options, including fonts and images. This is why this is one PR - you can't fix the one without fixing the other. What this means is that I also have to fix every widget. Should't take too long since the code is already written, I basically put every option that was implicit in theSomeWidget::new()
method and put it in theInto<WidgetBuilder>
method.Merging has to probably be done with
--merge-strategy=own
onreexport-api
. However, I first want to clean up my commits (supposing this branch will be merged).