david-vanderson / dvui

Other
431 stars 36 forks source link

Font theme rework #128

Closed VisenDev closed 2 months ago

VisenDev commented 2 months ago

Very experimental changes to separate a Font's scale from a Font's base_size

I am planning on migrating all prebuilt themes to json to make them easier to edit and easier to create new themes. This is one of several changes I am going to experiment with to make Themes easier to work with.

david-vanderson commented 2 months ago

Can you speak to the motivation here? What does having a base_size make possible or easier?

VisenDev commented 2 months ago

Can you speak to the motivation here? What does having a base_size make possible or easier?

The main thing is that it makes it easier to adjust the base font size when adding new themes. Originally, Themes had to specify specific font sizes for each instance of a font.

    .font_body = .{ .size = 13, .name = "Vera" },
    .font_heading = .{ .size = 13, .name = "VeraBd" },
    .font_caption = .{ .size = 10, .name = "Vera" },
    .font_caption_heading = .{ .size = 10, .name = "VeraBd" },

This made things difficult when trying to add new fonts for a theme, since if the default font size was too small, you would have to manually increment each .size = parameter to make them all scale equally.

This was unnecessarily difficult, so what I ended up doing on most of my themes was creating a size variable, and then initializing each font size using that size variable multiplied by a scaling factor. This allowed me to easily adjust the size of a font.

const size = 18;

pub const jungle = Theme{
    .font_body = .{ .size = size, .name = "Pixelify" },
    .font_heading = .{ .size = size, .name = "Pixelify" },
    .font_caption = .{ .size = size, .name = "Pixelify" },
    .font_caption_heading = .{ .size = size, .name = "Pixelify" },
    .font_title = .{ .size = size * 2, .name = "Pixelify" },
    .font_title_1 = .{ .size = size * 1.8, .name = "Pixelify" },
    .font_title_2 = .{ .size = size * 1.6, .name = "Pixelify" },
    .font_title_3 = .{ .size = size * 1.4, .name = "Pixelify" },
    .font_title_4 = .{ .size = size * 1.2, .name = "Pixelify" },
    //...
 };

At this point, the size variable doesn't really have anything to do with the theme. This base font size is really a property of the Font. The Theme just wants to be able to scale whatever that base size is.

Hence, its seems more straightforward to give each font a base_size parameter, and then scale that base_size up and down as necessary

This makes implementing Themes via json more straightforward because I cannot create a size variable for scaling in json as I can in zig. This means that every .size = field would have to be manually adjusted if the font was too big or too small.

Thus, it seems like swapping out the font size parameter for a font scale parameter makes fonts easier to work when implementing themes.

Hope that helps clarify. I'm not super tied to this change, I've just had this idea floating in my head for a while and wanted to test it out. Theres a couple things in Theme that have been kind of bugging me and this is one of them.

I mentioned this here as well https://github.com/david-vanderson/dvui/issues/81

VisenDev commented 2 months ago

An alternative approach would be to create a SerializableTheme struct that is specifically geared to make it easier to create new themes, and then convert that data into an actual dvui.Theme automatically

david-vanderson commented 2 months ago

Thanks for the explanation! Will think about this, it seems like the base_size is a property of the theme maybe. I certainly have wanted a way to scale all the fonts in a theme (see fontSizeAdd() which I'm not happy with) , but I can't put my finger on the right way to do it.

SerializableTheme is a good idea. Maybe that's the way to go.

VisenDev commented 2 months ago

I'll look into sending an alternative PR that does this instead