Last Comment Bug 695406 - Need reset front buffer if ContentType is different
: Need reset front buffer if ContentType is different
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla10
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on:
Blocks: 687372 689045 695275
  Show dependency treegraph
 
Reported: 2011-10-18 11:21 PDT by Oleg Romashin (:romaxa)
Modified: 2011-10-21 08:30 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Reset front buffer if content type has changed (3.93 KB, patch)
2011-10-18 11:23 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Reset all buffers if ContentType has changed (7.73 KB, patch)
2011-10-18 19:28 PDT, Matt Woodrow (:mattwoodrow)
cjones.bugs: review+
Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2011-10-18 11:21:28 PDT
Looks like we are not dropping front buffer if content type is different.
OGL/Basic, Shadow layers are affected
Comment 1 Oleg Romashin (:romaxa) 2011-10-18 11:23:51 PDT
Created attachment 567810 [details] [diff] [review]
Reset front buffer if content type has changed
Comment 2 Matt Woodrow (:mattwoodrow) 2011-10-18 19:19:57 PDT
I think we need to also include part of the changes from https://bugzilla.mozilla.org/attachment.cgi?id=567936&action=diff here.

This patch deletes the front buffer if the new front buffer has a different content type. We also need to detect the actual content type change on the child side, and create a new backbuffer before this code will ever be hit.

With this change, the BasicLayers changes are unnecessary since we will delete this buffer on the next frame paint. Though probably still a good idea.
Comment 3 Matt Woodrow (:mattwoodrow) 2011-10-18 19:28:48 PDT
Created attachment 567955 [details] [diff] [review]
Reset all buffers if ContentType has changed

Added in the check on the backbuffer side.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-18 23:30:01 PDT
Is this leading to bugs in the field (seems like it could!), and/or is it breaking bug 695275?  If the former, we need tests here.
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-18 23:32:44 PDT
Comment on attachment 567955 [details] [diff] [review]
Reset all buffers if ContentType has changed

I assume this obsoletes the previous patch?
Comment 6 Oleg Romashin (:romaxa) 2011-10-19 01:52:14 PDT
Comment on attachment 567810 [details] [diff] [review]
Reset front buffer if content type has changed

yes, it is obsolete
Comment 7 Matt Woodrow (:mattwoodrow) 2011-10-19 02:15:23 PDT
Just the latter.

I'm not sure how to test this exactly, nsDisplayImages are always marked as being transparent, we only hit this bug in the weird case that theres an empty visible region (and thus empty transparent region), but still decide to create the image layer.

Constructing a test that relies on this edge case doesn't sound great.
Comment 8 Matt Woodrow (:mattwoodrow) 2011-10-20 18:34:52 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/73dc291f8974
Comment 9 Marco Bonardo [::mak] 2011-10-21 02:21:42 PDT
https://hg.mozilla.org/mozilla-central/rev/73dc291f8974
Comment 10 Marco Bonardo [::mak] 2011-10-21 08:30:11 PDT
I backed out and relanded this to investigate an orange, thus the real final changeset is
https://hg.mozilla.org/mozilla-central/rev/75eaad17724f

Note You need to log in before you can comment on or make changes to this bug.