Closed
Bug 592870
Opened 14 years ago
Closed 14 years ago
Make personas caption buttons work with d3d9 layers
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta7+ |
People
(Reporter: Felipe, Assigned: Felipe)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
Joe and Bas tell me that with ded9 layers enabled, the region of the window's caption buttons are plain white (instead of transparent which would make the buttons visible).
Hopefully with the change in approach in this patch this will be solved. At the moment, we remove the caption button's area from our drawing region, but probably the surface is cleared to white first.
With this patch we don't touch the paint region. Instead we let all painting occur normally, and afterwards we paint transparent (using operator_clear) the area on the thebesSurface where the caption buttons are.
Assignee | ||
Comment 1•14 years ago
|
||
Yeah this doesn't work, the accelerated layers manager doesn't follow that code path.
Maybe we still should take this patch though, I don't know. I guess it will depend on what the code to support accelerated layers will need.
Assignee | ||
Comment 2•14 years ago
|
||
Now this is interesting, if we simply paint the DC with the black brush after all the painting happened, we can see the caption buttons. Probably the way the black brush works is setting the alpha to 0 on those pixels (which is ultimately what we want, but not sure if this is the right way).
This works for GDI, d2d, d3d9. The only problem is that the buttons flicker a bit specially when moving the mouse over the firefox button area.
Bas: any feedback? Is this working by lucky? If you have ideas on what's the correct way to do this let me know
Attachment #471303 -
Attachment is obsolete: true
Attachment #471379 -
Flags: feedback?(bas.schouten)
Comment 3•14 years ago
|
||
Comment on attachment 471379 [details] [diff] [review]
Second try
So, this working on D3D9, I believe is mostly because of luck. Same for D2D, GDI this is at least a better approach. What we should do to make this better for both GDI and D2D is do this from the context, i.e.
(pseudo code)
context->NewPath();
foreach (rectangle in region)
context->Rectangle(rect);
context->SetOperator(OPERATOR_CLEAR);
context->Fill();
I believe this is the best performing approach for D2D and GDI.
For hardware accelerated layers it becomes a bit trickier, since they don't follow this code at all. They shouldn't even be creating a cairo context there (see the LayerManager type switch statement).
Here we should probably just ensure from the users of the layermanager that the region of the caption buttons is ensured to be transparent. But I don't see why it wouldn't just be that, and why it would have white there in the first place.
Comment 4•14 years ago
|
||
Roc might have useful input.
Does the D3D9 layer manager need to clear its backbuffer to completely transparent before painting?
Assignee | ||
Comment 6•14 years ago
|
||
(In reply to comment #3)
> Comment on attachment 471379 [details] [diff] [review]
> Second try
>
> So, this working on D3D9, I believe is mostly because of luck. Same for D2D,
> GDI this is at least a better approach. What we should do to make this better
> for both GDI and D2D is do this from the context, i.e.
>
> (pseudo code)
>
> context->NewPath();
> foreach (rectangle in region)
> context->Rectangle(rect);
> context->SetOperator(OPERATOR_CLEAR);
> context->Fill();
>
> I believe this is the best performing approach for D2D and GDI.
Ok, that's what patch v1 does
FTR I did some quick measurements and AFAICT we very rarely create a complex region by excluding the caption buttons region, because the invalidated region usually does not intersect with that (unless something is forcing full repaints)
Updated•14 years ago
|
Blocks: d3d9-layers
Comment 7•14 years ago
|
||
Comment on attachment 471379 [details] [diff] [review]
Second try
We should only do the fillrect thing when gfxWindowsPlatform::GetPlatform()->GetRenderMode() == RENDER_GDI. Other than that I've figured out why it works, and it's fine.
What was actually breaking this for D3D9 layers is that the caption button area was never invalidated, and also never drawn to (GDI doesn't work here), the area would therefor keep the white, initial content of the window. Invalidating it with D3D9 layers will make it transparent, and blend properly.
Attachment #471379 -
Flags: feedback?(bas.schouten) → review+
Comment 8•14 years ago
|
||
Attachment #471379 -
Attachment is obsolete: true
Updated•14 years ago
|
Attachment #471949 -
Flags: review?(felipc)
Assignee | ||
Comment 9•14 years ago
|
||
Comment on attachment 471949 [details] [diff] [review]
Fix up comments from Bas
The original patch (patch v2) removed the code to subtract the caption buttons region from the event.region, and just always drew black on the DC, which made the buttons area transparent. If we then do that only for GDI, the caption buttons won't show up anymore with d2d or accelerated layers.
I think that the best to do would be:
if (basic layers) {
paint transparent on the thebesContext, as suggested on comment #3
} else {
event.region.Sub(mCaptionButtons);
Fix why this doesn't work on d3d9 (comment #7). note: this already works for OGL
}
P.S.: As the caption buttons always show white with d3d9 right now (personas or not), if the goal is really to enable d3d9 today, we can make the buttons work correctly without personas (by backing out the personas bug or applying patch v1 here) and break personas again, and then we set this as blocking b6
Attachment #471949 -
Flags: review?(felipc)
Updated•14 years ago
|
blocking2.0: --- → beta6+
Assignee | ||
Comment 11•14 years ago
|
||
So for now we can use the technique of clearing the DC which makes it work for d3d9, but not remove the event.region part yet because that makes basic layers + d2d flicker a lot.
After the invalidation issue is fixed I think we'll be able to remove this and do something like comment #9.
Attachment #471949 -
Attachment is obsolete: true
Attachment #472097 -
Flags: review?
Assignee | ||
Updated•14 years ago
|
Attachment #472097 -
Flags: review? → review?(jmuizelaar)
Comment 12•14 years ago
|
||
Comment on attachment 472097 [details] [diff] [review]
New patch
Good enough for now.
Attachment #472097 -
Flags: review?(jmuizelaar) → review+
Updated•14 years ago
|
Assignee: nobody → felipc
Comment 13•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 14•14 years ago
|
||
(In reply to comment #11)
> Created attachment 472097 [details] [diff] [review]
> New patch
>
> So for now we can use the technique of clearing the DC which makes it work for
> d3d9, but not remove the event.region part yet because that makes basic layers
> + d2d flicker a lot.
>
> After the invalidation issue is fixed I think we'll be able to remove this and
> do something like comment #9.
It's probably better if we do only do this once on the windows first paint. Every subsequent time we do this I believe is useless in the case of hardware accelerated layers. We'd need to verify this though.
You need to log in
before you can comment on or make changes to this bug.
Description
•