Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No fonts in project manager if using custom theme. #48477

Closed
autofool opened this issue May 5, 2021 · 50 comments
Closed

No fonts in project manager if using custom theme. #48477

autofool opened this issue May 5, 2021 · 50 comments

Comments

@autofool
Copy link

autofool commented May 5, 2021

Godot version:
3.3-stable

Issue description:
If I set custom fonts in editor theme resource, fonts won't show up in project manager. It is not even using any kind of fallback fonts.

Steps to reproduce:
Set theme file given below as custom theme in godot editor. Then press Ctlr+Shift+q to go back to project manager and it will not show fonts. Now go back to any project and copy theme in project folder. Edit it in inspector and just remove default font option. Now go back to project manager. Some of the font's come back except for few places. That's because theme is overriding more fonts in sub properties. If you remove all custom fonts. Everything will show properly .

Minimal reproduction project:
material.zip

@JestemStefan
Copy link
Contributor

This behaviour is documented.
obraz

The workaround is to apply theme to Control node

@YuriSizov
Copy link
Contributor

@JestemStefan Hey, which doc is this lifted from? I've actually addressed this propagation problem in one of my PRs so I would need to update the docs if/when it gets merged.

@JestemStefan
Copy link
Contributor

@pycbouh https://docs.godotengine.org/en/3.3/tutorials/gui/gui_skinning.html

@autofool
Copy link
Author

autofool commented May 5, 2021

@JestemStefan I'm sorry but I don't understand. How would applying theme to control node instance would affect Godot editor? Are we on same page? I'm not using theme for any gui scene for game but for editor itself.

@autofool
Copy link
Author

autofool commented May 5, 2021

20210505162901_REGION
This is the theme resource I'm talking about.
20210505162956_REGION
And that's project manager without any fonts.

@autofool
Copy link
Author

autofool commented May 5, 2021

I have set default font in theme resource. So it should use it globally if I have not set them somewhere.

@YuriSizov
Copy link
Contributor

This does seem like an independent issue, yes.

@JestemStefan
Copy link
Contributor

JestemStefan commented May 5, 2021

OK. I thought you are talking about custom theme for GUI. It seems like similar bug (propagation of the theme), but I didn't know about this one... maybe they are related? Someone more advanced need to look into it.

@Calinou

This comment has been minimized.

@autofool
Copy link
Author

autofool commented May 7, 2021

@Calinou Removing it would be really sad cause it's a really cool feature! I think custom theme for editor which uses the resource created by the editor is pretty fascinating. Plus, it gives users a prime example of Godot's capability to theme components without manually updating them. Hack we can even put some example on wiki so users can just browse editor with applied theme and check how it works without creating a whole scene with control skinning cause that has its own learning curve. This way new users who are still considering Godot can check out how GUI skinning works without actually learning Godot. And it should be recommended on features page (If custom theme is fixed).
Another thing is that we spend most of our time using editor and having some personal touch makes it more enjoyable. I personally take at least a whole day to set up any editor I'm going to use to my liking. On top of that default settings are a bit weird. Especially the contrast.
Contrast makes border panels more brighter and that's fine but there is no way to inverse contrast so that primary color is background color which is darkened. I like having my application look seamless with my desktop theme and contrast is really making things ugly.
If I want to achieve flat looks with it, its basically impossible due to the fact that Godot has very congested UI. It's only possible with manually tweaking the theme. Default option lower contrast just makes things harder to distinguish and confusing.
Improved custom theme is not an immediate issue that needs fixing asap but its fix is definitely one that would make day to day use of Godot more likeable. And it's not like custom theme is totally broken. It works pretty well just needs more robust application.

@Calinou
Copy link
Member

Calinou commented May 7, 2021

Plus, it gives users a prime example of Godot's capability to theme components without manually updating them.

We can have a demo project for that 🙂

I made a GUI Theming Override demo in the official demo projects repository last year. PRs are welcome to add theme selection to it (e.g. switching between a dark and light theme).

Contrast makes border panels more brighter and that's fine but there is no way to inverse contrast so that primary color is background color which is darkened. I like having my application look seamless with my desktop theme and contrast is really making things ugly.

That's something that could be exposed either by allowing negative contrast values or adding a new setting 🙂

I would prefer we improve the built-in theme customizations (to a reasonable extent). It's nice to advocate for the custom theme option, but until now, there is nobody to fix it...
Edit: pycbouh has volunteed to maintain this.

Also, the master branch just got a new default editor theme with a flatter, more modern design.

@autofool
Copy link
Author

autofool commented May 7, 2021

Also, the master branch just got a new default editor theme with a flatter, more modern design.
Wut!? Imma head out for a sec.

@autofool
Copy link
Author

autofool commented May 7, 2021

? Do you mean tres file?

@Calinou
Copy link
Member

Calinou commented May 7, 2021

? Do you mean tres file?

Yes, but I removed my comment as I realized the theme resource is in the MRP. (I was on mobile when I wrote the above comment.)

@autofool
Copy link
Author

autofool commented May 7, 2021

I have improved one if you want. Older one had colors that I hadn't modified.

@YuriSizov
Copy link
Contributor

It's nice to advocate for the custom theme option, but until now, there is nobody to fix it...

I take personal responsibility to make sure custom theme resources for the editor are in a working order. I see no reason why we wouldn't support a fully custom theme resource, if there are users willing to take time to create one. We use our internal Theme resource in an axiomatic way, so everything should be overridable. A few bugs are not a good reason to just cut it.

I would even argue we don't make our customization options more complicated. I like that Godot's theme can be adjusted in a very simplistic way. For everything more complex, there is (and will be) the Theme editor.

@Calinou
Copy link
Member

Calinou commented May 7, 2021

This is how the bug manifests itself with the master branch with the theme.tres included in the MRP:

image

Note that fonts are handled differently in the master branch. This is why the default project font seems to be used everywhere, and the bold font (used for project titles) is replaced with tofu instead.

In 3.3:

image

Only the bold font is missing. A custom theme could provide one by adding a font named bold to the EditorFonts class, but I'm not sure if this is possible from the current theme editor.

Edit: It's possible, see below.

@autofool
Copy link
Author

autofool commented May 7, 2021

Isn't option already there?
20210507191219_REGION

@Calinou
Copy link
Member

Calinou commented May 7, 2021

Isn't option already there?
20210507191219_REGION

doc_bold is the font for the editor help (built-in documentation), not the editor in general. bold seems to be the option you want here 🙂

I think the confusion stems from the fact that all fallback fonts are disabled as soon as one custom font is provided. We should keep providing fallback fonts for any fonts that weren't overridden.

@autofool
Copy link
Author

autofool commented May 7, 2021

Yah that's what I was talking about. cursor was on doc bold so that popup came up lol

@autofool
Copy link
Author

autofool commented May 7, 2021

I can quickly check the fallback theory but what is a safe options for fallback font? I mean which font should I use for fallback?

@Calinou
Copy link
Member

Calinou commented May 7, 2021

I can quickly check the fallback theory but what is a safe options for fallback font? I mean which font should I use for fallback?

You can use any font you want, but of course, you should use a bold font file if you want the font to be displayed in bold.

@autofool
Copy link
Author

autofool commented May 7, 2021

Umm I think Godot configuration broke. After I compiled and ran Godot from master branch. 3.3 one stopped applying theme to the project manager. I deleted the .config folder in home dir but issue still persists. Any solution to this?

@Calinou
Copy link
Member

Calinou commented May 7, 2021

Umm I think Godot configuration broke. After I compiled and ran Godot from master branch. 3.3 one stopped applying theme to the project manager. I deleted the .config folder in home dir but issue still persists. Any solution to this?

Godot 3 and 4 use different configuration files (see File paths in Godot projects for their location). This is specifically done to avoid breaking the editor settings every time you switch versions.

Also, in both 3.3 and master, I noticed that the custom theme only applies after restarting the editor. The setting hint should be changed to indicate this 🙂

@autofool
Copy link
Author

@pycbouh how does Godot find project icons then?

@YuriSizov
Copy link
Contributor

YuriSizov commented May 13, 2021

By manually adjusting the res:// path and manually loading an Image and creating an ImageTexture resource:

		Ref<Image> img;
		img.instance();
		Error err = img->load(item.icon.replace_first("res://", item.path + "/"));
		if (err == OK) {
			img->resize(default_icon->get_width(), default_icon->get_height(), Image::INTERPOLATE_LANCZOS);
			Ref<ImageTexture> it = memnew(ImageTexture);
			it->create_from_image(img);
			icon = it;
		}

@autofool
Copy link
Author

I just double checked and Godot's theme file, when asked to set by editor, does not use path in context to any project because theme file can be anywhere in the system. So a resource which is not in any project should not use 'res://' for its sub resources. Aren't sub resources embedded in '.res' or '.tres' file?

@Calinou
Copy link
Member

Calinou commented May 13, 2021

Aren't sub resources embedded in '.res' or '.tres' file?

They can be, but it's not a requirement. Subresources can also be saved to external .res/.tres files which are then referenced in the main resource file.

@YuriSizov
Copy link
Contributor

YuriSizov commented May 13, 2021

The theme resource you've presented has an example of both: images are packed into it as a byte array, but fonts (DynamicFontData to be precise) are external:

[ext_resource path="res://fonts/opensans/OpenSans-Bold.ttf" type="DynamicFontData" id=1]
[ext_resource path="res://fonts/iosevka/iosevka-extended.ttf" type="DynamicFontData" id=2]
[ext_resource path="res://fonts/opensans/OpenSans-Regular.ttf" type="DynamicFontData" id=3]

@autofool
Copy link
Author

autofool commented Sep 23, 2021

@pycbouh Is there an update on this? Project Manager is still "fontless" in 3.3.3.stable.
One off topic question since you are the one working on theme related stuff. There's this label in debugger bottom tab call "Stack Frames". It would be nice if I could know which element is that? That's the only one I am not able to change.

20210923212337_SCREEN

@YuriSizov
Copy link
Contributor

YuriSizov commented Sep 23, 2021

Is there an update on this? Project Manager is still "fontless" in 3.3.3.stable.

We have no system for referencing resources globally (or relatively to each other for that matter), they can only be linked within a project with project-relative paths. So I don't think we can easily find a solution in the near future.

I've not looked into the fallback issue, which should be solvable. In the end, we should probably disable custom themes in the PM due to these problems. Or at least sanitize it to remove external resources somehow (so that colors, for example, are retained, but not fonts or icons).

Edit: I guess, an alternative solution can be in some way to "pack" the entire theme into one resource file, so there are no external resources? I don't remember, though, if that would cause issues for fonts or not.

One off topic question since you are the one working on theme related stuff. There's this label in debugger bottom tab call "Stack Frames". It would be nice if I could know which element is that? That's the only one I am not able to change.

Isn't it just a header of a Tree control?

PS. Quite nice flat design you've got there! Good job.

@autofool
Copy link
Author

autofool commented Sep 23, 2021

We have no system for referencing resources globally (or relatively to each other for that matter), they can only be linked within a project with project-relative paths.

What about forcing of fonts as embedded resource in theme file?

In the end, we should probably disable custom themes in the PM due to these problems.

That's sounds good. A user doesn't spend too much time in Project Manager anyway. Most of the session is spent in editor so having only editor affected by theme is reasonable.

Isn't it just a header of a Tree control?

Unfortunately, current theme resource has no property to modify header of tree control. As you can see in bottom left corner, I have already customized whole tree control. Thanks!

@YuriSizov
Copy link
Contributor

Unfortunately, current theme resource has no property to modify header of tree control. As you can see in bottom left corner, I have already customized whole tree control. Thanks!

I checked, and it's definitely the title row. It seems to be drawn using the following properties:

  • title_button_color

  • title_button_font

  • title_button_normal

  • title_button_hover

  • title_button_pressed

The relevant code is here:

https://github.com/godotengine/godot/blob/3.3/scene/gui/tree.cpp#L3057-L3072

@autofool
Copy link
Author

Got it! Thanks a bunch!

@autofool
Copy link
Author

@pycbouh I just tried 3.4 beta and Project Manager has default fonts! It's perfect!
20210923234316_WINDOW

@YuriSizov
Copy link
Contributor

Isn't project name still missing? Or is that an issue with your theme? 3.4 supports partial editor themes, so it should fill the gaps for missing stuff, but I'm not sure it's the complete cure.

@autofool
Copy link
Author

Whoops! Totally missed that in excitement but that's an improvement for sure. Only titles are missing cause they use Bold fonts? It was produced in master:

This is how the bug manifests itself with the master branch with the theme.tres included in the MRP:

image

Note that fonts are handled differently in the master branch. This is why the default project font seems to be used everywhere, and the bold font (used for project titles) is replaced with tofu instead.

In 3.3:

image

Only the bold font is missing. A custom theme could provide one by adding a font named bold to the EditorFonts class, but I'm not sure if this is possible from the current theme editor.

Edit: It's possible, see below.

@autofool
Copy link
Author

Do note that the in 3.3.stable; second shot has all the fonts except title but current 3.3.3.stable has no fonts at all.

@autofool
Copy link
Author

This is 3.3.3 stable official build:

20210923235159_SCREEN

@autofool
Copy link
Author

Sadly, I can not confirm whether 3.4 fills the gap left by custom theme cause beta5 crashes when custom theme is applied. #51648 should fix it though. Then, I'll confirm again if default fonts work for project title and everything then I don't think we need more solid solution. At least not any time soon. But if 3.4 fails to show fonts then I guess we can keep this issue reopened for a reminder.

@YuriSizov
Copy link
Contributor

#51648 is in beta5...

@autofool
Copy link
Author

Whoops! Should I reopen #52530 then?

@YuriSizov
Copy link
Contributor

YuriSizov commented Sep 23, 2021

If you think it's the same crash, sure. But make sure to update the reproduction steps for 3.4. And if you can, attach the resource that causes the problem.

In the meantime, should this one still be open perhaps?

@autofool
Copy link
Author

Yes but I'll reopen tomorrow. I'm commenting from phone app and I can't find the option.

@Cammymoop
Copy link
Contributor

This was fixed by #51648

@akien-mga akien-mga added this to the 3.4 milestone Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants