Overview
Accessible Sudoku is designed to allow users their choice of look and feel to ensure readability and to reduced eye strain ... or to enhance it, user's choice really.
To that end the Options menu allows the user to adjust font, color, and scale data for use throughout the game. These themes are then saved to the user-space SQLite database.
During the process of converting from Godot 3 to 4 Sudoku's theme system hit several hurdles, including now taking about 15 seconds to apply any change. Some were quick fixes, other took the use of external profilers to figure out.
Sudoku's theme system bugs
- Any theme change requires about 15 seconds to apply.
- Loading scenes is significantly slower.
- Color data read in from the DB is incorrect.
Color data read in from the DB is incorrect.
The culprit here is the Color class to_html function. The change can be seen in the relevant section of the 3.6 and 4.1 documentation.
3.6:
Returns the color's HTML hexadecimal color string in ARGB format (ex:
ff34f822).
4.1:
Returns the color converted to an HTML hexadecimal color String in RGBA format, without the hash (
#) prefix.
ARGB -> RGBA
Converting from ARGB to RGBA is easy enough, just pop the first two chars and push them to the back of the string.
theme_in[color_item] = "%s%s" % [
theme_in[color_item].right(-2),
theme_in[color_item].left(2)
]The difficult part is identifying when it's necessary to apply.
Save file versions
Starting with this next release, Accessible Sudoku will begin saving the application version to the files saved in user-space. Since this is a new implementation it'll be enough to check if the version was loaded as to whether the ARGB -> RGBA logic is needed to ingest themes.
Loading scenes is significantly slower.
This bug ties into the theme change lag directly, but was much easier to fix being the read (CRUD) operation only.
Godot built-in debugger and profiler
Godot's built-in debugger is great! It lets you step though the code and examine the stack in all sorts of great ways. Maybe my vim-development days are showing through as I did not use a lot of great debuggers in the past, but I'm thoroughly impressed with this one.
Stepping through the code is easy, the monitors helped identify orphan nodes causing memory leaks (see the update in 20230630's devlog), and just all around makes the dev process easier.
But... it can't catch everything. It doesn't show much of what the engine is doing on the back-end unless you know the correct signals to catch and watch for.
func _notification(what) -> void:
match what:
Control.NOTIFICATION_THEME_CHANGED:
Log.silly("%s: Theme changed." % ["Options: _notification"])With this bit of code I was able to see that loading a scene would send quite a few notifications for theme changes.
Reduce theme complexity
Godot works in a fairly strict hierarchy. The root window contains the autoloads / singletons and the scene. The Scene is structured in a parent to child relationship and settings are passed down unbroken chains (Control vs Node).
So first step is to reduce the number of places actually setting a theme and rely on the inheritance.
Next a new theme was created to house just the options being changed and nothing superfluous.
These steps were the low hanging fruit that resolved bug #2 well enough. On to the remaining CrUD operations.
Any theme change requires about 15 seconds to apply.
After restructuring theme assignments to reduce redundancy and maximize inheritance theme change lag was down from 15 seconds to about 5. Not bad, but certainly not very user friendly.
Godot's built-in debugger / profiler was not very illuminating, only showing a bit of lag in the physics processing time.
Using the Time class timing checks were added into the theme modification code and consistently showing a short (sub 1 second) run time, so what's going on?
Enter the external profilers which need a build of Godot that includes debugging symbols. A bit of building with the Godot repo and we're ready to launch the profiler.
Huh?
Yeah, that's what I said. It took a while for me to sort of understand what this graph is saying, but here it goes.
This graph aggregates and displays system calls used during the profiling with system calls being shown on top of their calling system call (e.x., Main::iteration() forks off to RenderingServerDefault::_draw(bool, double) and SceneTree::physics_process(double)).
ThemeOwner::propagate_theme_changed(node*, Node*, bool, bool)
The biggest chunk of the flame graph is consumed by what seems to be a recursive call to propagate_theme_changed. This seems to be called on each theme change to propagate the change down through the hierachy.
By removing the extranneous theme assignments and relying more on inheritance, the recursion issue was alleviated but not removed.
As the theme system got more complex the propagation lag got worse.
Theme Buffer
After some helpful dialog with a user in the Godot Engine matrix channel (#godotengine:matrix.org) and some digging through the Theme class documentation a solution was implemented.
At the start of the configure() function a new Theme is instantiated. Calculated changes from the Options interface are applied to the new / non-active theme. At the end, the calculated theme is merged into the active one with Theme's merge_with() function resulting far fewer Control.NOTIFICATION_THEME_CHANGED signals.
Next steps
From here, further refinements were included by stepping through the code with the debugger to identify any remaining theme-related redunancy.
Modifying the default font on the buffer theme doesn't seem to overwrite the default theme after merge_with() is called, so after the merge configure() updates the default font on the theme directly; resulting in two Control.NOTIFICATION_THEME_CHANGED notifications.
Comments