Duotone: defer metadata init - #74591

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: sgomes <sergiomdgomes@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

While WP_Duotone_Gutenberg doesn't exist in Core, if this approach is found desirable we may want to port it to WP_Duotone.

I'm happy to either add a backport entry or add the No Core Sync Required label to the PR, depending on what you think is the right thing to do.

Flaky tests detected in 0133986.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/21139484574
📝 Reported issues:

• [Flaky Test] should allow setting page order to zero and negative values #74483 in /test/e2e/specs/site-editor/pages.spec.js

Hey @t-hamano! 👋 You seem to have prior experience with this feature, so is there any chance you could review this PR, or point me to a different reviewer? Thank you!

@sgomes Thanks for the PR!

Unfortunately, I wasn't involved much in the development of this feature 😄 Initializing on demand certainly seems like it would help, but I'd like to hear what others think. @WordPress/gutenberg-core Also, @westonruter is knowledgeable about performance improvement.

Thank you so much, @t-hamano, and sorry for misunderstanding and dragging you into this! 😅

@sgomes, could you also submit a core-backport PR, referencing this README?

1. Create a core ticket with the gutenberg-merge keyword: Examples of tickets
2. Make changes to the WP_Duotone class and submit a PR
3. Attach a backport changelog file to this PR

@westonruter : I believe I addressed all your comments, please take a look!

@sgomes, could you also submit a core-backport PR

Thanks, @t-hamano! I'll go ahead and do that, then 👍

It turns out I'm not the only one to have thought of this 🙂

When looking at backporting these changes, I noticed that @spacedmonkey already did something similar in Core back in July 2023 with this PR: WordPress/wordpress-develop@fc53820

I'm going to see if I can replicate his changes in WP_Duotone_Gutenberg, to bring Core and Gutenberg a bit closer, instead of relying on my original work. I'll update the PR if successful.

I'm going to close this PR and start a new one based on @spacedmonkey's aforementioned changes. There is more to it than the added caching, with changes to some method signatures and behaviour that help further in avoiding unnecessary work.

Sorry for missing it in the first place, and thank you for the reviews so far!

I created #74748 with the changes ported from Core.

I'm actually wondering if WP_Duotone_Gutenberg is even necessary.

I realized that there have been many changes made exclusively in WP_Duotone or WP_Duotone_Gutenberg that weren't properly synchronized, resulting in differences between the two classes.

The WP_Duotone class is present in every WordPress core release that Gutenberg supports. And the Duotone feature is now mostly stable. I believe there won't be any issues if I remove WP_Duotone_Gutenberg completely, but am I missing something?

If we want to add new features to Duotone in the future, would it be possible to add an override class to the lib/compat/wordpress-x.x directory and override only specific methods?

I'm actually wondering if WP_Duotone_Gutenberg is even necessary.

That's a great question, @t-hamano, and I think it bears further investigation!

I'm coming at this problem from a performance perspective (WP_Duotone_Gutenberg initialisation kept showing up in some traces I was looking at), so I don't have much experience with this feature. I just wanted to see if I could make it faster, and deferring the work it was doing seemed like a no-brainer 🙂

I'll see what I can dig up as to why WP_Duotone_Gutenberg came to be in the first place, and how easy it would be to drop it entirely in favour of WP_Duotone 👍

@t-hamano / @westonruter:

I tested removing WP_Duotone_Gutenberg and falling back to WP_Duotone, and I think it's generally close to working. It does work correctly for classic themes, with the styles and the filter being output correctly.

However, it fails to work for block themes, which handle the block styles differently. While in classic themes the Duotone block styles are output by Duotone itself, in block themes this is handled by wp_enqueue_stored_styles during wp_enqueue_scripts. When Gutenberg removes the default filter and replaces it with its own, the Core Duotone styles aren't output, because they were stored in the Core store, and not the Gutenberg one. This means that the image gets the CSS class, and the SVG filter is present in the page, but the styles that define that class (and thus tie the two together) are missing.

I'm not sure how to proceed here, other than keeping WP_Duotone_Gutenberg and porting the changes from Core into it (#74748). I'm open to any suggestions!

I'm afraid I wasn't involved in the development of the Duotone functionality, so I don't have any wisdom to offer.

For what it's worth, this is Gemini's analysis of the issue:

The Core Duotone functionality is missing because of a discrepancy between how styles are stored by Core and how they are retrieved by the Gutenberg plugin's replacement function.

1. Storage: When Core's WP_Duotone generates block styles, it calls wp_style_engine_get_stylesheet_from_css_rules with the context block-supports. This stores the CSS rules in the Core style engine store (WP_Style_Engine_CSS_Rules_Store).

2. Retrieval: The function gutenberg_enqueue_stored_styles replaces Core's version. For the primary "core" styles (which include the block-supports context), it uses gutenberg_style_engine_get_stylesheet_from_context.

3. The Gap: Unlike the Core version, the Gutenberg-prefixed style engine functions only look into the Gutenberg style engine store (WP_Style_Engine_CSS_Rules_Store_Gutenberg). Because WP_Duotone (running in Core) has stored its rules in the Core store, the Gutenberg function never "sees" them when it queries the block-supports context.

4. Partial Merging: While gutenberg_enqueue_stored_styles does include logic to merge additional stores from Core (lines 332-335 in gutenberg/lib/client-assets.php), it explicitly skips this merging for any context keys defined in $core_styles_keys (line 338), which includes block-supports. Since Duotone styles are stored under that specific key in the Core store, they are effectively orphaned when the Gutenberg plugin is active.

Thank you, @westonruter, that does match my own analysis. So it seems that the only way we could rely on Core's implementation would be if we were to move to Core for all block-supports styles, which involves many features beyond Duotone and is probably infeasible.

I'll keep working on merging the Core changes into Gutenberg (#74748) instead 👍