Re: gtk_style_context_get()



This is a pipe dream, I imagine, but would it be possible to get people to cache their stuff only inside size_allocate(); or draw();, and just do something dumb like lie to the user and warn if they do it outside of one of those paths?

I think doing drawing of the DND window outside of the frame clock is a bit much --  we should be synchronized to that in any case.

I haven't investigated how much breakage something like that would cause.

On Thu, Jan 29, 2015 at 11:51 PM, Benjamin Otte <otte gnome org> wrote:
Hey,

Here's a problem I'm currently thinking about and would like input on. I discussed it with Matthias on #gtk+ today and he suggested I'd reach out to more people. I've included the log below and formatted it for clarity so that it reads like a QA-style interview. I hope it is not too inconvenient to read.

<Company> the question we're trying to answer is this: What CSS values does GtkStyleContext return?
<Company> this question is relevant because every time we change the CSS tree, we need to (potentially) update the value

<Company> 3.0.0 used the approach of immediately updating everything once the css tree changed somehow
<Company> so if you did: context.add_class("a"); context.add_class("b"); context.add_class("c");
<Company> it would recompute the CSS 3x
<Company> which gets real slow really fast

<Company> in 3.4 or so I changed that to always return a stale value
<Company> until we validated the style via gtk_container_idle_sizer() (when the frame clock runs)
<Company> so from that point on everything was fast, but the values were sometimes incorrect

<mclasen> wouldn't you want to mark the values as stale, and force a recompute when somebody asks for a value ?
<Company> ideally, you would
<Company> the reason I did not do that was the style-updated signal
<Company> when should that be emitted?
<Company> if we emit it once a context gets invalid, we go back to 3.0 performance
<Company> because every widget queries values in the style-updated handler...

<mclasen> you could do some freeze/thaw to batch the 3 emissions from 3 add_class calls
<Company> I don't want to add freeze/thaw calls though, everybody would get those wrong

<mclasen> who needs to listen to style-updated anyway ?
<mclasen> other than the gtk redraw machinery
<Company> mostly just that
<Company> GtkWidget needs to update the pango context
<Company> GtkImage needs to release the cached surface
<Company> things like that
<Company> and of course we need to queue_draw() or queue_resize()

<Company> here's another caveat: computing the CSS values requires computing the CSS values of the parent

<mclasen> is that on the person changing the style context, or do we do that automatically ?
<Company> GtkWidget::style-updated does that
<Company> based on the GtkCssAffects settings of the changed style properties

<mclasen> my mental model of this is that css values are used when drawing happens, so they need to be current at that point
<mclasen> and drawing happens when the frame clock comes around to tell us
<Company> yes
<Company> when the frame clock triggers, we (1) recompute styles, (2) size_allocate everything pending, (3) draw

<mclasen> whether css values are up-to-date at other points in the frame cycle should not be important
<mclasen> except for those 3rd party users who randomly poke at style contexts...
<Company> but it is important
<Company> otherwise you get shrinking gnome terminals and white firefox text
<Company> the gnome terminal case is actually rather complex to get right with the "should not be important" assumption
<Company> because it needs to compute a bunch of values based on multiple different widgets
<Company> and it needs to compute those before size allocation happens
<Company> in short: It needs to happen in style_updated() at the latest - but which style-updated, when it depends on multiple widgets?

<halfline> can't you just give the right value any time the user asks for it (updating the cache for that property if it's stale), but also keep style-updated where it happens now ?
<Company> that's one thing we could do
<Company> there's many solutions to the problem(s)
<Company> I haven't reached the point yet where want to talk about solutions
<Company> so far I'm trying to list the problem(s) :)

<mclasen> 'before size allocation' sounds like a global thing though ? like a point in the frame cycle
<mclasen> as long as we keep information about which styles are up-to-date, doing on-the-spot validation for people who call gtk_style_context_get_foo outside of regular frame-based drawing may just fine ?
<mclasen> at least, only the people doing the poking are paying the price of extra recomputations then

<Company> lemme do a short jsfiddle to show you how the web does it
<Company> http://jsfiddle.net/nbt2myzb/1/
<Company> I hope that example is simple enough
<Company> you wanna understand why uncommenting that 1 line of JS makes the animation restart
<Company> (disclaimer: I'm using firefox, no idea if webkit does things differently somehow)

<mclasen> does it ?
<mclasen> I seem to get blue when I click, which slowly fades to yellow
<mclasen> no restarts that I can observe
<Company> uncomment the line in the JS
<Company> then press the "run" button
<mclasen> ah, ok
<mclasen> now it restarts

<Company> you understand why it does that now?
<mclasen> no, why does it do that ?
<mclasen> I see that you are causing it to get the computed value of the color when I click
<mclasen> but how that relates to restarting the animation is not clear to me
<Company> right
<Company> getting the color causes a recompute of the CSS
<Company> but the "animate" class isn't set, so the style has no animation
<Company> so all running animations get deleted
<Company> then we add the animate class again, so the animation is created again
<Company> of course, recreation means restart
<mclasen> a bit surprising
<Company> yes
<Company> getting a value can have side effects
<Company> otoh: You always get the correct CSS value for the current state of the CSS tree

<mclasen> why is it correct,though ?
<mclasen> wouldn't you expect to get the value of the color at the point you happen to be in in the aninmation ?
<mclasen> ah no, you animate background, not color
<Company> no, because you removed the animate class right above
<Company> so the value is not animated when you query it
<Company> (you can use x.backgroundColor if you want to)
<mclasen> right

<mclasen> so, whats the upshot for our situation - discourage poking at css values, because recomputing them will have ugly side effects ?
<mclasen> where 'discourage' could include dropping ::style-updated
<Company> our situation gets even more funky
<Company> because our situation also has the question: "when do we emit style-updated"?
<Company> we cannot drop it
<mclasen> or only emit it at strategic times
<Company> we have to emit queue_resize() at some point ;)
<mclasen> clearly

<Company> here's an updated version: http://jsfiddle.net/nbt2myzb/2/

<mclasen> on the web, you don't have a ::style-updated equivalent, right ?
<Company> I haven't found it
<mclasen> I mean, we need to connect style changes to our size allocate machinery, but do we have to let apps hook onto that ?
<Company> if we want to report correct values (and I think we want to) we can't do anything about restarting animations or other such side effects
<Company> the only thing we can influence is at what point we emit style_updated
<Company> we clearly do not want to emit it when the style gets invalidated
<Company> we might want to emit it when the style gets validated
<Company> or we might want to emit it only when the next frame happens

<Company> if we do it when the style gets validated, getting a property might actually have the side effect of emitting a signal - or multiple signals, because validating a style requires validating the parent style
<mclasen> is that so unusual ?
<mclasen> you are getting a _computed_ value, so you shouldn't be too surprised that some computation happens...
<mclasen> maybe call it compute_value() instead of get_computed_value() ?
<Company> that a getter emits a signal? That is very unusual I think
<Company> especially because those getters are called from render functions and other places
<Company> say, if you start a dnd operation, you render text, which will update the labels's style which will update the toplevel's style which will cause I don't know what GtkWindow::style-updated does
<Company> all just because we rendered a drag icon for text
<Company> now granted, all of this would have run with the next frame clock update anyway, but the fact that it's happening earlier might be scary

<Company> the 2nd option is problematic, too
<Company> because if we only emit style-updated at frame boundaries
<Company> the widget will not invalidate its own caches until that point
<Company> so you will start the drag operation with old fonts (because we only update the pango context in style-updated)

<mclasen> why does that drag operation have to render out-of-frame ? because we pass the rendered icon to the compositor ?
<Company> we render the icon in the dnd event handler
<Company> this is all mostly hypothetical btw, because nobody sane changes the widget font when dnd starts
<Company> it's just the most realistic example i can come up with that illustrates potential problems
<mclasen> not so sure, actually - we had proposals to do cut/paste in nautilus by keeping grayed out 'ghosts' of the selected files in the list until they get dropped elsewhere

<Company> here's another possibile idea:
<Company> We could add a GtkWidget::style-invalidated signal
<Company> and then we have 2 signals: One when styles go invalid and one when styles go valid again
<Company> not sure if this buys us anything, but we could add it
<mclasen> and if you react to the invalidated signal by getting some value, you'll cause the updated signal to be emitted ?
<Company> yeah
<Company> and you kill performance

<mclasen> who would use the one vs the other, and what would it buy us ?
<Company> the idea would be to use the invalidate signal to drop caches
<Company> so that say GtkImage drops the cached surface
<Company> the "rendering dnd images" example again
<Company> you can't just render the cached image, you need to first make sure the CSS values didn't change

<Company> of, course you could have an emit_style_updated_if_necessary() function, too
<mclasen> gtk_style_context_validate () ?
<Company> yeah, like that
<Company> depending on when we emit style-updated, gtk_style_context_get_color() might be good enough for that purpose

<Company> of course, if someone does context.remove_class ("foo"); context.add_class ("foo"); you don't wanna drop caches
<Company> or rather: you'd like if you didn't do that
<Company> and GtkWidget::style-invalidated would not handle that
<Company> and that happening is a common theme
<Company> with my_widget_update_style() { remove_all_styles (); apply_relevant_styles(); }
<Company> and then call my_widget_update_style() from various setters that may or may not change the actual styles

<Company> I'll leave you with that for now - if you have more questions or an opinion into which solution you prefer, please get back to me
<Company> otherwise I'll make up my mind at some point (I don't have an opinion yet)

_______________________________________________
gtk-devel-list mailing list
gtk-devel-list gnome org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list




--
  Jasper


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]