You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This may be a little bit niche, but I think there can be some improvement done to Glide Path Cache or (GPC for short).
Right now, GPC store does not take into account if you change assets.image_manipulation.cache. When you change cache, the storage path for manipulated images also change. But GPC is not aware of storage path location, it is only aware of the relative path from the storage root. So when you change cache from true to false, it will try to read manipulated image in storage where that manipulated image does not exist yet.
Now, this is mostly applicable to the glide:data_url tag, as it reads directly image content directly from cache to generate data URL. Regular glide tag seems to be not affected as it just generates URLs, the approach is different there.
..aand you also have to be unaware, like me, that glide:clear also will clear this GPC. And there won't be no problem. But I still feel like this is a bit un-intuitive and could be maybe more user friendly, especially when normal Glide tag usage works as intended and does not break, while glide:data_uri does.
To reproduce a problematic scenario:
Do glide:clear for both assets.image_manipulation.cache => true and assets.image_manipulation.cache => false, so both glide caches are squeaky clean
Change config to assets.image_manipulation.cache => true
Generate data URI with {{ glide:data_url :src="assets_field" w="20" tag="true" }} and observe that image works
Switch config back to assets.image_manipulation.cache => false
Load the page again and you should see that the image is not working anymore
The code responsible for generating data URI from Glide tag class is this:
The data URI generation fails silently and the error gets logged. I oppose to having such generous try .. catch over here but maybe the team has a good reason that I am not aware of.
Regardless the exception Unable to read file from location gets thrown when I remove the try .. catch. You can observe the stack trace here: https://flareapp.io/share/w5BOZe6m#F80
The discovery of this issue originally comes from responsive images addon, this issue here. This addon generates placeholder image to be inlined, and basically mimics/copies the same code to achieve the same thing as glide:data_uri.
The code we have for it is the following right now:
You can see that I have worked around this problem with a cheesy strategy. I have added a cosmetic, arbitrary parameter based on the cache config value to trick ImageGenerator class to generate a new manipulated image, if the user does change the cache value.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
This may be a little bit niche, but I think there can be some improvement done to Glide Path Cache or (GPC for short).
Right now, GPC store does not take into account if you change
assets.image_manipulation.cache
. When you changecache
, the storage path for manipulated images also change. But GPC is not aware of storage path location, it is only aware of the relative path from the storage root. So when you changecache
fromtrue
tofalse
, it will try to read manipulated image in storage where that manipulated image does not exist yet.Now, this is mostly applicable to the
glide:data_url
tag, as it reads directly image content directly from cache to generate data URL. Regularglide
tag seems to be not affected as it just generates URLs, the approach is different there...aand you also have to be unaware, like me, that
glide:clear
also will clear this GPC. And there won't be no problem. But I still feel like this is a bit un-intuitive and could be maybe more user friendly, especially when normal Glide tag usage works as intended and does not break, whileglide:data_uri
does.To reproduce a problematic scenario:
glide:clear
for bothassets.image_manipulation.cache => true
andassets.image_manipulation.cache => false
, so both glide caches are squeaky cleanassets.image_manipulation.cache => true
{{ glide:data_url :src="assets_field" w="20" tag="true" }}
and observe that image worksassets.image_manipulation.cache => false
The code responsible for generating data URI from
Glide
tag class is this:The data URI generation fails silently and the error gets logged. I oppose to having such generous
try .. catch
over here but maybe the team has a good reason that I am not aware of.Regardless the exception
Unable to read file from location
gets thrown when I remove thetry .. catch
. You can observe the stack trace here: https://flareapp.io/share/w5BOZe6m#F80The discovery of this issue originally comes from responsive images addon, this issue here. This addon generates placeholder image to be inlined, and basically mimics/copies the same code to achieve the same thing as
glide:data_uri
.The code we have for it is the following right now:
You can see that I have worked around this problem with a cheesy strategy. I have added a cosmetic, arbitrary parameter based on the
cache
config value to trickImageGenerator
class to generate a new manipulated image, if the user does change thecache
value.GPC remembers manipulated images based on the manipulation params. From
ImageGenerator::generateByAsset
:I am unsure whether this should have been a bug report or feature request, so I made this into a discussion instead.
Beta Was this translation helpful? Give feedback.
All reactions