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