Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced Clutter.Image with St.ImageContent (fixes GNOME 48) #261

Open
wants to merge 4 commits into
base: version-51
Choose a base branch
from

Conversation

high1
Copy link

@high1 high1 commented Feb 4, 2025

According to https://gitlab.gnome.org/jrahmatzadeh/gjs-guide/-/commit/b150648e7ad460a9cff3afeecdbdb6b8d17550ab, Clutter.Image will not work in GNOME 48 - replaced it with St.ImageContent, recommended by the porting guide. This is backwards compatible with GNOME Shell 45 and higher.

GJS

Clutter.Image

Clutter.Image has been removed and you can use St.ImageContent instead.
There is no need to version check for St.ImageContent
as it's already available in GNOME Shell 45 and higher.

@high1 high1 changed the title Replaced Clutter.Image with St.ImageContent Replaced Clutter.Image with St.ImageContent (fixes GNOME 48) Feb 4, 2025
@high1
Copy link
Author

high1 commented Feb 6, 2025

Unfortunately, while this worked for a short while, this was changed in st/image-content: Take a CoglContext on set_bytes/set_data functions so that St.ImageContent now requires a Cogl.Context parameter, and I can't find an example how to do this.

EDIT: An explanation: They've removed Clutter.Image in Gnome 48, and the instructions clearly state that extensions should use St.ImageContent in it's place, which worked right out of the box when I tried it - and it will work for Gnome 45 to 47. Unfortunately, the latest update to Gnome 48 beta (which I got early from Arch Gnome-Unstable repo) changed St.ImageContent in Gnome48 to require CoglContext as a parameter - breaking the previous compatibility.

@high1 high1 closed this Feb 6, 2025
@high1 high1 reopened this Feb 6, 2025
@high1
Copy link
Author

high1 commented Feb 6, 2025

OK, found out how to get the context - but I think that this means that there will have to be a new version of the extension for Gnome 48 - because St.ImageContent now takes Cogl.Context, which it didn't in Gnome 45 to 47. So, given that - changed the target branch to version-51. Maybe the new release could cover both 45-47 and 48, but this call to instantiate a new instance of St.ImageContent will differ for 45 to 57 and 48.

image

@high1 high1 force-pushed the replace-clutterimage-with-stimagecontent branch from 99c3842 to da6aa2e Compare February 6, 2025 11:02
@high1 high1 changed the base branch from master to version-51 February 6, 2025 11:03
@neffo
Copy link
Owner

neffo commented Feb 6, 2025

Looks good so far, let me know when you think it's ready to merge. Looks like we are pretty good for GNOME 48 other than this issue.

@high1
Copy link
Author

high1 commented Feb 7, 2025

image
Tested a change that would allow the extension to work on 45-57 and 48. Looks good to me, not sure if you agree with the latest code update.
EDIT: After Gnome upgrade
image
LGTM, if you agree with the version check .

@neffo
Copy link
Owner

neffo commented Feb 7, 2025

I've not actually tested this PR yet, sorry. Is the work around required, the porting guide suggests that it isn't for GNOME 45+.

https://gjs.guide/extensions/upgrading/gnome-shell-48.html#clutter-image

@high1
Copy link
Author

high1 commented Feb 7, 2025

The workaround is needed (if I'm not missing something obvious) if you'd like the extension to work for 45-47 and 48 at the same time in the next version. Clutter.Image or St.ImageContent - doesn't matter - there will have to be a version check. This just uses St.ImageContent since that way most of the code can be shared between the versions - i.e. the guide doesn't take into account that GNOME 48 broke the compatibility with the previous versions - if it didn't, you could've just replaced Clutter.Image with St.ImageContent (like I did in the first iteration) and call it a day.
I've added the link to the change in GNOME 48 in previous posts, but here's the screenshot where compatibility was broken
image. The Guide recommendation just stops working after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants