Allow to set opacity on moz-tree-image

RESOLVED FIXED in mozilla7

Status

()

Core
Widget
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

Trunk
mozilla7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-places])

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

6 years ago
I'm trying to reproduce the classic "cut" effect on a tree, where the icon for a certain row gets some transparency. Unfortunately ::moz-tree-image opacity is ignored.

I see most of the drawing is done in nsTreeBodyFrame::PaintImage ( http://mxr.mozilla.org/mozilla-central/source/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp#3380 )
I can most likely get opacity from imageContext->GetStyleDisplay()->mOpacity, but I have no idea how to apply it to the drawing code.

Any hint or example I could look at?
(Assignee)

Updated

6 years ago
OS: Windows 7 → All
Hardware: x86 → All
(Assignee)

Updated

6 years ago
Blocks: 416459
gfxContext::PushGroup/gfxContext::PopGroupToSource/gfxContext::Paint.
(Assignee)

Comment 2

6 years ago
OMGITWORKS
(Assignee)

Comment 3

6 years ago
Created attachment 539469 [details] [diff] [review]
patch v1.0

Something like this?
Attachment #539469 - Flags: review?(roc)
Comment on attachment 539469 [details] [diff] [review]
patch v1.0

Review of attachment 539469 [details] [diff] [review]:
-----------------------------------------------------------------

I'd prefer you to not add this to nsLayoutUtils::DrawImage/DrawImageInternal. Instead, just do the push/pop in nsTreeBodyFrame, using aRenderingContext->ThebesContext().

::: layout/base/nsLayoutUtils.cpp
@@ +3290,5 @@
>                 aImageFlags);
> +
> +  if (aOpacity != 1.0f) {
> +    ctx->PopGroupToSource();
> +    ctx->SetOperator(gfxContext::OPERATOR_OVER);

You don't need to set the operator here, it should already be OVER.
(Assignee)

Updated

6 years ago
Assignee: nobody → mak77
Status: NEW → ASSIGNED
(Assignee)

Comment 5

6 years ago
Created attachment 539479 [details] [diff] [review]
patch v1.1

Good idea, the change is much more restricted.
Attachment #539469 - Attachment is obsolete: true
Attachment #539469 - Flags: review?(roc)
Attachment #539479 - Flags: review?(roc)
Comment on attachment 539479 [details] [diff] [review]
patch v1.1

Review of attachment 539479 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!

A reftest for this would be appreciated.
Attachment #539479 - Flags: review?(roc) → review+
(Assignee)

Comment 7

6 years ago
Created attachment 539787 [details] [diff] [review]
reftest v1.0

This one should work, one tree has a fully opaque black icon with opacity: 0.5, the other one has a half opaque black icon with no opacity.
Locally I get:
REFTEST TEST-PASS | file:///c:/mozilla/places/layout/reftests/bugs/664127-1.xul | image comparison (==)
sounds like working, but being my first reftest I don't believe it :)
Attachment #539787 - Flags: review?(roc)
This is probably a little too fragile, there's no guarantee that the opacity calculations on the platform match the values you calculated in your image.

I'd just use a solid black image in both files and put opacity:0.5 on the <tree> in your reference file.
(Assignee)

Comment 9

6 years ago
OK I'll try, btw the previous test (the one with black icon and opacity: 0.5) for some reason was not showing the icon at all in the test on tryserver, while it works here locally...
(Assignee)

Comment 10

6 years ago
Well, not completely true, it worked fine on linux opt. Maybe it doesn't wait enough for the icon to show.
(Assignee)

Comment 11

6 years ago
Created attachment 540023 [details] [diff] [review]
reftest v1.1

I was able to reproduce the failure locally and it was indeed a snapshot timing thing. I added a 50ms delay and now it works, will push to try.
Attachment #539787 - Attachment is obsolete: true
Attachment #539787 - Flags: review?(roc)
(In reply to comment #11)
> I added a 50ms delay and now it works
Just giving you a chance to avoid filing another orangefactor bug...
(Assignee)

Comment 13

6 years ago
fwiw, it still fails on Mac try (still waiting other platforms), I thought reftests were easier to get right!  In this case there is absolutely nothing I could listen at to be sure it is ready for the comparison, so I don't see how I could handle it differently than with a reasonable timeout (it is used in other reftests as well), unless you have suggestions.  Painting a simple icon can't be that much slow!
(Assignee)

Comment 14

6 years ago
So, it is permagreen on all platforms but Mac, where it's permaorange. The reference image (<tree> with opacity: 0.5) is completely blank there, while the reftest is correct. If I open the reftest on my Mac it looks correct though (I still have to run the reftest harness though). I don't think this Mac thing is due to the timeout though, it failed at all tried and respins.
Comment on attachment 540023 [details] [diff] [review]
reftest v1.1

You're not running any js code, so you shouldn't need the timeout at all.  By default the reftest framework waits until a test is loaded and then paints the page.  What rendering do you get if you remove the reftest-wait class and the onload handlers?
Attachment #540023 - Flags: review-
(Assignee)

Comment 16

6 years ago
the tree icon is painted asynchronously, the usual waiting is not good enough for it. If I remove the timeout the icon is not painted.
(Assignee)

Comment 17

6 years ago
hm, looks like my reftest was fine, and it just found a Mac regression, on Firefox 4.0.1 I can open my reftest and I see a half-opaque tree, on current Firefox 7 I see an empty page :(
(Assignee)

Comment 18

6 years ago
To clarify:
- After a brief irc discussion with Ehsan, painting is synchronous, but most likely the asynchronous part is fetching the image from the channel.  I'll try adding a second image to the page to see if this fetching can become cached so I can get rid of the timeout.
- The Mac regression is a problem apart, I'm looking for a regression range and will file a separate bug for it (we may be able to cover 2 bugs with a single reftest!)
(Assignee)

Updated

6 years ago
Depends on: 665133
(Assignee)

Comment 19

6 years ago
Created attachment 540229 [details] [diff] [review]
reftest v1.2

This doesn't need a timeout, the <img> preloads the image so that the tree has it in cache, to allow for this I need an external stylesheet. Using a real image also bypasses bug 665133, so that one will need its own reftest.
Note I can't preload a datauri since datauris are not cached.
Already passed try (http://tbpl.mozilla.org/?tree=Try&rev=329996f60c77)
Attachment #540023 - Attachment is obsolete: true
Attachment #540229 - Flags: review?(roc)
Comment on attachment 540229 [details] [diff] [review]
reftest v1.2

Review of attachment 540229 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #540229 - Flags: review?(roc) → review+
(Assignee)

Updated

6 years ago
No longer depends on: 665133
(Assignee)

Comment 21

6 years ago
baking on Places:
http://hg.mozilla.org/projects/places/rev/ed60c3abdb71
http://hg.mozilla.org/projects/places/rev/26bda006334c
Whiteboard: [fixed-in-places]
(Assignee)

Comment 22

6 years ago
http://hg.mozilla.org/mozilla-central/rev/ed60c3abdb71
http://hg.mozilla.org/mozilla-central/rev/26bda006334c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla7

Comment 23

6 years ago
Hi.
Is there a way to verify this?
Thanks
You need to log in before you can comment on or make changes to this bug.