Skip to content

Conversation

StamLord
Copy link

This PR is an attempt to address #91814

As suggested by @Calinou, I based my solution on the approach used in visual_shader_editor_plugin.cpp.

Changes included:

  • Added a local Ref object I instantiate in AnimationTreeEditor, instantiated in the constructor
  • Populated the theme in a new update_theme() function
  • Called update_theme() from the constructor, immediately after creating the theme, since AnimationTreeEditor does not have an _update_graph function like in VisualShaderEditor
  • Applied the theme to AnimationTreeEditor itself (set_theme) so that all child AnimationTreeEditorPlugin inherit it.
    Applying it to the individual plugins also works, but seems redundant.

This replaces the default font with and MSDF font that scale better.

MSDF_fix

@StamLord StamLord requested a review from a team as a code owner September 12, 2025 17:29
@Calinou Calinou added bug topic:editor topic:animation cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release labels Sep 13, 2025
@Calinou Calinou added this to the 4.6 milestone Sep 13, 2025
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

Some feedback:

  • The font should only applied to GraphEdit, not the rest of the AnimationTree editor. This prevents fonts such as this one from looking different (and blurrier) than the rest of the editor:
image

In other words, MSDF fonts should only be used where the zoom level can dynamically be changed.

@StamLord StamLord requested a review from a team as a code owner September 14, 2025 10:21
@StamLord StamLord force-pushed the animationtree-msdf-fix branch from f52812d to c9e3355 Compare September 14, 2025 10:24
@StamLord
Copy link
Author

Thanks @Calinou I moved the fix to GraphEdit and it seems to work:
image

Can you shed some light on how GraphEdit relates to the AnimationTreeEditor? Is it the parent or is there some inheritance going on that I didn't find?

@StamLord StamLord force-pushed the animationtree-msdf-fix branch 9 times, most recently from aab9483 to 31d6bbf Compare September 14, 2025 16:26
@StamLord
Copy link
Author

I've been wrestling with the pre commit hooks a bit.

  • I wrapped the headers, GraphEdit::update_theme and the set_theme call in #ifdef TOOLS_ENABLED so it will not fail non-editor builds
    *I added a null check of the editor singleton inside set_theme since it failed on headless Linux builds (maybe due to singleton not being ready yet?)

I'd appreciate someone taking a closer look at my workarounds since maybe there's a simple way to handle this.

Question: since GraphEdit is a usuable node in a shipped game, should the default font be msdf outside the editor as well?
MSDF will look better by default but we might want to leave theme control up to the user.

@Calinou
Copy link
Member

Calinou commented Sep 14, 2025

Files in scene/ should not have dependencies on editor code, so I'm afraid this isn't the correct way to fix the issue. The previous approach was more logical from a technical perspective; it just needed to be more scoped to avoid affecting the entire AnimationTree editor (and affect just the GraphEdit that's within).

Question: since GraphEdit is a usuable node in a shipped game

GraphEdit/GraphNode are also usable in exported projects, e.g. in Material Maker.

@StamLord StamLord force-pushed the animationtree-msdf-fix branch from 31d6bbf to d2847c8 Compare September 19, 2025 11:23
@StamLord StamLord force-pushed the animationtree-msdf-fix branch from d2847c8 to d81558b Compare September 19, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release topic:animation topic:editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants