After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 703909 - st-theme-node: Missing corners
st-theme-node: Missing corners
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2013-07-10 01:39 UTC by Florian Müllner
Modified: 2013-09-07 00:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
st-theme-node: Don't update resources on each paint (1.07 KB, patch)
2013-07-10 01:39 UTC, Florian Müllner
committed Details | Review
st-theme-node: Update corner textures on allocation size changes (1.55 KB, patch)
2013-07-10 01:40 UTC, Florian Müllner
committed Details | Review

Description Florian Müllner 2013-07-10 01:39:49 UTC
This is a regression from the box-shadow optimizations - the most obvious reproducer for me:
 (1) bring up the alt-tab popup
 (2) dismiss it
 (3) bring it up again

In (3), the rounded corners from both the popup and the item highlight are no longer drawn.
Comment 1 Florian Müllner 2013-07-10 01:39:54 UTC
Created attachment 248788 [details] [review]
st-theme-node: Don't update resources on each paint

Up to commit 318283fc705bd9e9, resources were only updated when
the allocated size had changed. There is no good reason to change
this for theme nodes without box shadows.
Comment 2 Florian Müllner 2013-07-10 01:40:00 UTC
Created attachment 248789 [details] [review]
st-theme-node: Update corner textures on allocation size changes

Commit 318283fc70 optimized box-shadow rendering by not recreating
shadow materials on every allocation change. Other handles cannot
be reused and are updated regularly, however the patch missed the
cached corner materials - while those can be reused, we still need
to ensure that the currently used paint state references them.
Comment 3 Florian Müllner 2013-07-10 01:41:24 UTC
(The first patch is not necessary for fixing the issue, I just don't understand why the condition has been removed in the first place ...)
Comment 4 Lionel Landwerlin 2013-07-10 14:50:57 UTC
Review of attachment 248788 [details] [review]:

Looks good to me.
Comment 5 Lionel Landwerlin 2013-07-10 14:52:04 UTC
Review of attachment 248789 [details] [review]:

Yeah, I wondered why the corners where put into the PaintState, we can probably leave them in the ThemeNode and not even have reference on them.
Comment 6 Lionel Landwerlin 2013-07-10 15:00:33 UTC
Review of attachment 248789 [details] [review]:

I think this is fine too, thanks for spotting this.
Comment 7 Florian Müllner 2013-07-10 16:02:53 UTC
Attachment 248788 [details] pushed as f9b3247 - st-theme-node: Don't update resources on each paint
Attachment 248789 [details] pushed as e031a5d - st-theme-node: Update corner textures on allocation size changes

(In reply to comment #5)
> Yeah, I wondered why the corners were put into the PaintState, we can probably
> leave them in the ThemeNode and not even have reference on them.

Well, it's a corner case (sic!), but the corners used for drawing might be different from the ones requested by the theme node if the allocation is too small to accomodate the original corners without overlapping.

However not taking a reference in always looking them up in paint might be an option - the calculations for the actual radii/colors and a hash table lookup are not terribly expensive ...

I'll leave the bug open, as I'm still seeing some missing corners occasionally (the focus indication after opening and dismissing the run dialog a couple of times); oddly enough, reverting my first patch doesn't help, though it should cause the corners to be actually looked up each paint.
Comment 8 Florian Müllner 2013-09-07 00:00:58 UTC
(In reply to comment #7)
> I'll leave the bug open, as I'm still seeing some missing corners occasionally

I haven't done so in a while, so looks fixed to me.