Closed
Bug 664127
Opened 14 years ago
Closed 13 years ago
Allow to set opacity on moz-tree-image
Categories
(Core :: Widget, defect)
Core
Widget
Tracking
()
RESOLVED
FIXED
mozilla7
People
(Reporter: mak, Assigned: mak)
References
Details
(Whiteboard: [fixed-in-places])
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
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•14 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
gfxContext::PushGroup/gfxContext::PopGroupToSource/gfxContext::Paint.
Assignee | ||
Comment 2•14 years ago
|
||
OMGITWORKS
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•14 years ago
|
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•14 years ago
|
||
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•14 years ago
|
||
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•13 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•13 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•13 years ago
|
||
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)
Comment 12•13 years ago
|
||
(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•13 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•13 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 15•13 years ago
|
||
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•13 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•13 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•13 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 | ||
Comment 19•13 years ago
|
||
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 | ||
Comment 21•13 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•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ed60c3abdb71
http://hg.mozilla.org/mozilla-central/rev/26bda006334c
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Comment 23•13 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.
Description
•