RESOLVED FIXED 97466
[chromium] adding a page overlay causes endless update loop when accelerated compositing is used 
https://bugs.webkit.org/show_bug.cgi?id=97466
Summary [chromium] adding a page overlay causes endless update loop when accelerated ...
Andrey Kosyakov
Reported 2012-09-24 10:49:34 PDT
0. In chromium, navigate to about:flags, assure "GPU compositing on all pages" and "FPS counter" are enabled 1. Navigate to about:blank 2. Open DevTools 3. Observe FPS graph is still 4. Switch to Elements panel and hover <body>, so bluish page overlay is visible 5. Observe we're doing a steady 60fps
Attachments
Patch (1.45 KB, patch)
2012-09-24 10:55 PDT, Andrey Kosyakov 		no flags
DetailsFormatted DiffDiff
Patch (1.45 KB, patch)
2012-09-24 11:00 PDT, Andrey Kosyakov 		jamesr: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Andrey Kosyakov
Comment 1 2012-09-24 10:55:56 PDT
Created attachment 165410 [details] Patch
Andrey Kosyakov
Comment 2 2012-09-24 11:00:12 PDT
Created attachment 165411 [details] Patch
James Robinson
Comment 3 2012-09-24 11:27:38 PDT
In https://bugs.webkit.org/show_bug.cgi?id=73235#c9, Vsevolod Vlasov said: "Inspector needs invalidating on each composite because highlighted element might have moved, but I am not sure you ever need to invalidate dimming layer at all.". Is that no longer the case? Is inspector taking care of invalidating the overlay when the highlighted elements moves nowadays?
Andrey Kosyakov
Comment 4 2012-09-24 13:08:50 PDT
(In reply to comment #3) > In https://bugs.webkit.org/show_bug.cgi?id=73235#c9, Vsevolod Vlasov said: "Inspector needs invalidating on each composite because highlighted element might have moved, but I am not sure you ever need to invalidate dimming layer at all.". Is that no longer the case? Is inspector taking care of invalidating the overlay when the highlighted elements moves nowadays? We stopped updating the element position during animation some time ago, when we migrated the overlay to be an HTML page, so I don't think it's relevant now -- basically, all it does now is invalidating the view, while we only update element position on change of the element being inspected. I guess, if we ever need this again, the right place would be from InspectorOverlay::update(), based on invalidations generated from within the layout() of the overlay page. The best place to call update() would perhaps be animate() rather than composite(), as it's where the highlighted element may be moved and it would be early enough to process the change in the element position within the same frame.
James Robinson
Comment 5 2012-09-24 17:07:49 PDT
OK. Is there any way to have tests for the behavior we want for inspector so instead of guessing we can just run the test and see if we need it or not?
Andrey Kosyakov
Comment 6 2012-09-25 08:24:02 PDT
Committed r129511: <http://trac.webkit.org/changeset/129511>
Andrey Kosyakov
Comment 7 2012-09-25 08:30:53 PDT
(In reply to comment #5) > OK. Is there any way to have tests for the behavior we want for inspector so instead of guessing we can just run the test and see if we need it or not? We can do that at least for current behavior (i.e. when we do not update overlay on each animation) -- this would basically be a regression test for this bug. Adding it as bug 97567.
Note You need to log in before you can comment on or make changes to this bug.