Repository navigation
Avoid duplicate execution of setting up image editor when determining dominant color and transparency #373
Description
Follow-up to #19 / #282: Currently, both the dominant_color_get_dominant_color() and the dominant_color_has_transparency() functions execute the same logic to get the attachment file and instantiate the correct editor based on it.
This is unnecessary and somewhat wasteful since the only place where the two functions are relevant is in immediately subsequent calls for when the image has just been uploaded.
In addition to that, the two functions suggest by their public nature and clear names that they are okay to be called by third-party developers - which is not the case. These two functions perform expensive actions, and they must not be called "lightly". Their current name and documentation may lead to plugin developers calling them in the frontend to determine the dominant color and transparency of an image, but that is definitely not the intended use-case.
If we want to provide an easy-to-use API to get the dominant color and transparency of an image, these functions should rely on the already computed and stored values in the attachment's metadata. The way that the current functions behave is a no go for a "public API" though, and per the initial point of this issue is also slightly wasteful for performance during the upload process of an image.
Here's what I propose:
- Combine the two current functions
dominant_color_get_dominant_color()anddominant_color_has_transparency()into a single function marked as@access privatethat computes both values and returns them, or aWP_Error. - Introduce two new functions
dominant_color_get_dominant_color()anddominant_color_has_transparency()instead, which can truly serve as an "API" that is okay to be called from third-party developers. Both of these functions should simply look at the attachment's metadata and returned the stored value if set, or something likenullotherwise.
Activity
spacedmonkey commentedon Jun 15, 2022
I personally don't think we should worry about it, until we get to core patch. Once in core we will need a filter to change the editor class, that is a just because we are in the plugin ATM. Once in core. I recommend, adding transparency and dominiant color to
wp_generate_attachment_metadatafunction.I also through, that it might be better to change the signuration of the
dominant_color_get_dominant_color()and thedominant_color_has_transparency()function, to be inline withwp_create_image_subsizesthat passes a file path as well.felixarntz commentedon Jun 15, 2022
@spacedmonkey It's a fair point that we could also fix this when we get to the core patch, but given this is a feature project, it makes sense to implement things as close as possible to a future core implementation. And having "easy-to-use" public-API-like functions that actually run expensive operations is not great - that logic should be internalized IMO.
For that reason, I'd advise implementing this enhancement as part of the plugin.
pbearne commentedon Jun 16, 2022
Pull request here #381