GNOME Bugzilla – Bug 659159
scrollbar is faded
Last modified: 2011-09-19 16:51:45 UTC
Created attachment 196648 [details] screenshot You can see that half of the scrollbar is faded out.
Created attachment 196748 [details] [review] st-scroll-view-fade: Add the padding to the scrollbar size We need to take the padding into account otherwise we will end up fading the scrollbar when padding is present.
Comment on attachment 196748 [details] [review] st-scroll-view-fade: Add the padding to the scrollbar size looks right, although it makes the variable name (scrollbar_width) somewhat wrong... it might make more sense to pass in fade_area_width or something instead.
Created attachment 196750 [details] [review] st-scroll-view-fade: Take the padding into account when excluding the scrollbar Instead of passing the scrollbar_width/height to the shader compute the faded area (while taking the padding into account) and pass that instead, so we don't end up fading the scrollbar.
Created attachment 196760 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader Instead of doing complex computations in the shader just pass in the correct fade area (taking padding, scrollbars and rtl into account) and just work with that in the shader. That fixes a bug where we would fade the scrollbar when padding is present.
Created attachment 196764 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader Instead of doing complex computations in the shader just pass in the correct fade area (taking padding, scrollbars and rtl into account) and just work with that in the shader. That fixes a bug where we would fade the scrollbar when padding is present. --- Remove cruft ...
Comment on attachment 196764 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader shouldn't the height and width variables in the shader be replaced with fade_area references? hm... and no idea how that affects the x and y computations... oh, hm, and you need to take borders into account too... you should just use clutter_actor_get_allocation() and st_theme_node_get_content_box(). I'm thinking we should add some stuff to one of the scrolling tests in tests/interactive/ to verify that this all works...
(In reply to comment #6) > (From update of attachment 196764 [details] [review]) > shouldn't the height and width variables in the shader be replaced with > fade_area references? > > hm... and no idea how that affects the x and y computations... No the shader gets the whole texture so I need the whole width/height to be able to compute the current position in pixels (x and y). > oh, hm, and you need to take borders into account too... you should just use > clutter_actor_get_allocation() and st_theme_node_get_content_box(). OK. > I'm thinking we should add some stuff to one of the scrolling tests in > tests/interactive/ to verify that this all works... Should I just add padding / borders there or ... ?
Created attachment 196768 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader Instead of doing complex computations in the shader just pass in the correct fade area (taking padding, scrollbars and rtl into account) and just work with that in the shader. That fixes a bug where we would fade the scrollbar when padding is present. --- So this one should be using the correct content_box (and thus fixing the bug). Still missing the test though.
Created attachment 196770 [details] [review] scroll-view-sizing: Add tests for padding / borders Add tests to verify that the fade works fine with borders and padding.
Created attachment 196791 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader Instead of doing complex computations in the shader just pass in the correct fade area (taking padding, scrollbars and rtl into account) and just work with that in the shader. That fixes a bug where we would fade the scrollbar when padding is present. --- Fix comment.
Created attachment 196806 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader Instead of doing complex computations in the shader just pass in the correct fade area (taking padding, scrollbars and rtl into account) and just work with that in the shader. That fixes a bug where we would fade the scrollbar when padding is present. --- Remove no longer needed / used rtl_uniform.
Comment on attachment 196806 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader playing around with scroll-view-sizing: - regardless of borders/padding, the shadow overlaps the scrollbar by one pixel - with borders enabled and padding disabled, the shadow overlaps the left border entirely, a top shadow overlaps the top border entirely, and a bottom shadow overlaps the bottom border by one pixel.
Created attachment 196958 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader Instead of doing complex computations in the shader just pass in the correct fade area (taking padding, scrollbars and rtl into account) and just work with that in the shader. That fixes a bug where we would fade the scrollbar when padding is present.
(In reply to comment #13) > Created an attachment (id=196958) [details] [review] > st-scroll-view-fade: Pass a precomputed fade area to the shader > > Instead of doing complex computations in the shader just pass in the correct > fade area (taking padding, scrollbars and rtl into account) and just work > with that in the shader. > > That fixes a bug where we would fade the scrollbar when padding is present. Forgot to add a comment this should fix the issues pointed out in #c12
Comment on attachment 196958 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader ok, still one problem that I didn't notice before: when the scrollbar is not at the top of the scroll area, there is a partial shadow (with an abrupt cutoff) at the bottom (in addition to the correct shadow on the top)
Created attachment 196966 [details] [review] st-scroll-view-fade: Pass a precomputed fade area to the shader Instead of doing complex computations in the shader just pass in the correct fade area (taking padding, scrollbars and rtl into account) and just work with that in the shader. That fixes a bug where we would fade the scrollbar when padding is present. --- *) Fix handling of overlapping
Attachment 196770 [details] pushed as 247ad9d - scroll-view-sizing: Add tests for padding / borders Attachment 196966 [details] pushed as 09fe12d - st-scroll-view-fade: Pass a precomputed fade area to the shader