Skip to content

Conversation

@lenemter
Copy link
Member

@lenemter lenemter commented Mar 24, 2025

New clean branch that should be easier to review

Window switcher is used as an example of how this effect can be used because Danielle showed some interest in blurring it: elementary/granite#747 (comment)

Can be rebase merged

@lenemter lenemter requested a review from a team March 24, 2025 21:20
@lenemter lenemter force-pushed the lenemter/background-blur-effect branch 3 times, most recently from 50d77bd to 8061863 Compare March 24, 2025 21:27
@lenemter lenemter requested a review from a team March 24, 2025 21:46
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for UX, but still needs code review :)

@danirabbit danirabbit requested a review from a team March 24, 2025 22:03
@lenemter lenemter force-pushed the lenemter/background-blur-effect branch 9 times, most recently from 6f4a0ed to f942cd9 Compare March 29, 2025 10:27
public void set_uniform_1f (int uniform_location, float value);
public void set_uniform_1i (int uniform_location, int value);
public void set_uniform_float (int uniform_location, int n_components, int count, float value);
public void set_uniform_float (int uniform_location, int n_components, int count, float *value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this is a pointer? According to the vapi docs if possible you should use the vala provided syntax to make the semantics clearer: https://docs.vala.dev/developer-guides/bindings/writing-a-vapi-manually/07-00-binding-a-c-function-s-parameter-and-return-types/07-07-pointers.html

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pointer to the start of array of float values in C header. I don't know if there's other way to pass it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I guess you could pass it as an array? IIUC the array length would be n_components * count? In that case you'd probably have to do array_length = false in the vapi. On the other hand the deprecated Cogl.Program has a method with the same name and annotates the count as array length.
But i don't know enough about shaders and that stuff to say which it is 😅
Also the vapi seems to be generated by vapigen (at least that's what it says in the header) so ig we should probably put the change in the metadata file? https://docs.vala.dev/developer-guides/bindings/generating-a-vapi-with-gobject-introspection.html#fixing-vapi-generation-with-metadata

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it in the vapi file for now. I always assumed our vapi files were written by hand since the meson script in the vapi folder never worked for me. I guess we should ask @tintou about where should this change be

@lenemter lenemter force-pushed the lenemter/background-blur-effect branch from f942cd9 to a686740 Compare April 17, 2025 19:30
@danirabbit danirabbit requested a review from leolost2605 May 7, 2025 00:21
@lenemter lenemter force-pushed the lenemter/background-blur-effect branch from a686740 to 0c16fe6 Compare May 8, 2025 11:49
Copy link
Member

@leolost2605 leolost2605 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna be honest no clue how the blur works but structure LGTM. It has a bit of a weird frame in light mode on a light window but ig we can ignore that for now 🤷

@lenemter lenemter force-pushed the lenemter/background-blur-effect branch from 0c16fe6 to 353c813 Compare May 14, 2025 10:13
@lenemter
Copy link
Member Author

@leolost2605 I don't see anything weird here. Can you send a screenshot please?

@leolost2605
Copy link
Member

I dont know if that's intended but it looks like the background overflows the frame especially in the corners
Screenshot from 2025-05-14 12 39 22@2x

@lenemter
Copy link
Member Author

@leolost2605 That's a ShadowEffect issue. I'll look into fixing it later

@lenemter lenemter force-pushed the lenemter/background-blur-effect branch from 353c813 to 27ad901 Compare May 14, 2025 12:36
@zeebok zeebok added the Conflicts Has conflicts with the target branch label Jun 15, 2025
@danirabbit
Copy link
Member

@lenemter what's blocking here? Would be great to get this in for 8.1 :)

@danirabbit danirabbit moved this to Needs review in OS 8.1.0 Jul 22, 2025
@lenemter lenemter force-pushed the lenemter/background-blur-effect branch 2 times, most recently from 27ad901 to 9431076 Compare July 23, 2025 03:33
@lenemter lenemter removed the Conflicts Has conflicts with the target branch label Jul 23, 2025
@lenemter lenemter force-pushed the lenemter/background-blur-effect branch 3 times, most recently from 4049181 to 9f9aafc Compare July 23, 2025 05:00
@lenemter lenemter force-pushed the lenemter/background-blur-effect branch from 9f9aafc to 812e6e6 Compare July 23, 2025 05:04
@lenemter lenemter merged commit 66f07c3 into main Jul 23, 2025
6 checks passed
@lenemter lenemter deleted the lenemter/background-blur-effect branch July 23, 2025 05:07
@github-project-automation github-project-automation bot moved this from Needs review to Done in OS 8.1.0 Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants