Closed Bug 113232 Opened 23 years ago Closed 20 years ago

support transparent non-rectangular chrome windows

Categories

(Core :: XUL, defect, P1)

x86
All
defect

Tracking

()

RESOLVED FIXED
mozilla1.4alpha

People

(Reporter: rvj, Assigned: roc)

References

Details

(Whiteboard: [adt2][fix])

Attachments

(10 files, 19 obsolete files)

27.77 KB, application/zip
Details
22.89 KB, application/pdf
Details
7.41 KB, image/png
Details
334 bytes, application/vnd.mozilla.xul+xml
Details
13.27 KB, image/gif
Details
403 bytes, application/vnd.mozilla.xul+xml
Details
13.94 KB, image/png
Details
68.91 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
4.22 KB, patch
pavlov
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
1.71 KB, patch
roc
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
will the <window/> element  support background-color:transparent allowing the 
desktop to be revealed?

It a couple of years since I originally raised this in the newsgroups and 
altough there seems enthusiasm for this, Im not sure if it is possible to 
implement.


The idea is that if a window background-color is transparent and background is 
set to a gif with transparent regions, then the desktop will be visible through 
the transparent regions  e.g. an artists palette

Or is there another solution that would allow the design of irregular (non-
rectangular) windows.????
If we do this we should also do it for <iframe> and <menupopup>
Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → Future
Yipes. This would require (in Windows 2000&XP) making the entire window
WS_EX_LAYERED, having Mozilla manage which areas of itself are transparent, and
changing the rendering code to render to a buffer instead of a window, which
Windows will then composite. This would only work in 2000&XP, although I'm sure
Mac OSX is capable of something similar.

Another nice feature of the WS_EX_LAYERED style is that irregular windows are
easy, so theme makers will be happy.

As a fall-back, for systems that don't support real transparency, the Desktop
image could be rendered behind the translucent content instead.
--> default owner
Assignee: hyatt → jaggernaut
Target Milestone: Future → ---
Target Milestone: --- → Future
I'm researching ways to implement this on win32.

For the irregular window shapes, we need to calculate a polygon which defines
the outer edge of the window, and then call SetWindowRgn on the native window. 
Windows then takes care of clipping our pixels and keeping out mouse events on
the transparent regions.  
Assignee: jaggernaut → hewitt
Summary: will window support background-color:transparent revealing desktop → support transparent non-rect chrome windows
Keywords: nsbeta1
*** Bug 177716 has been marked as a duplicate of this bug. ***
Nav triage team: nsbeta1+/adt2
Keywords: nsbeta1nsbeta1+
Whiteboard: [adt2]
-> me
Assignee: hewitt → varga
targetting
Status: NEW → ASSIGNED
OS: other → All
Priority: -- → P1
Target Milestone: Future → mozilla1.3alpha
doesnt this require using .png files for themes?
And i think this will make Mozilla cool
We're targetting on rounded corners for now.
Attached patch patch v0.1 (obsolete) — Splinter Review
initial patch
Attached file testcase (obsolete) —
testcase
Summary: support transparent non-rect chrome windows → support transparent non-rectangular chrome windows
Attached patch new patch (obsolete) — Splinter Review
this one implements support for non-cotiguous regions too
Attachment #108282 - Attachment is obsolete: true
Attached file new testcase
Attachment #108283 - Attachment is obsolete: true
Joe, could you r/sr ?
Attachment #108992 - Flags: review?(glazman)
Attached patch better patch (obsolete) — Splinter Review
Attachment #108992 - Attachment is obsolete: true
Attachment #108992 - Flags: review?(glazman)
cc'ing bryner for a review
Attachment #109166 - Flags: superreview?(peterv)
Comment on attachment 109166 [details] [diff] [review]
better patch

I really don't like the dependency of widget on layout (and the style system)
that this patch creates.  It seems like it would be better to define
nsWindowRegion in gfx or widget.

It would be nice to avoid the copy of the points in the gtk widget code, but it
looks like gtk uses 16-bit types for the coordinates.
Attachment #109166 - Flags: review-
Attached patch fix (obsolete) — Splinter Review
nsWindowRegion moved to gfx/
Attachment #109166 - Attachment is obsolete: true
Attachment #109166 - Flags: superreview?(peterv)
Attachment #109176 - Flags: superreview?(peterv)
Daniel, could you take a look at the patch ?
I fixed the gotos (not sure why I used them before)
Comment on attachment 109176 [details] [diff] [review]
fix

r=bryner on the gfx, xpfe, and widget changes, with one comment:  please make
sure to use the MPL license with the correct copyright year on the new files.
Attachment #109176 - Flags: review+
Why do you need a new CSS property? Can't we just implement the existing CSS
properties better? We could make 'background:transparent' and
'-moz-border-radius' on <window> (and popups) actually work. That could actually
be more convenient and more powerful than what you currently have.
notice, that this patch implements irregular shapes, not just rounded corners
Still, with a transparent popup you can put whatever shape you like inside the
popup. Your testcases are basically just images; it'd be much easier for authors
to simply use a transparent image than to generate complicated regions.

Here's what Jan and I agreed: we won't introduce any new CSS properties. We will
instead make 'background-color:transparent' on XUL <window>s Do The Right Thing.

Thus, I propose adding three nsIWidget APIs:
-- nsIWidget::SetWindowTransparency(PRBool aTransparent); makes a top-level
window support transparency (we call this from layout when we detect
background-color:transparent on a top-level XUL window)
-- PRBool nsIWidget::GetWindowTransparency()
-- nsIWidget::UpdateTransparentWindowAlpha(nsRect aUpdateArea, PRUint8*
aAlphaValues); sets the alpha values for the window's pixels (the parameter is a
w x h array of 8-bit alpha values) (this gets called by the view manager for a
transparent widget before copying the off-screen buffer to the window's
rendering context)

I have promised to modify the view manager so that when the window is
transparent, the view manager will compute the alpha values for every painted
pixel and send them to the widget.

To implement this on Windows ME/W2K/WinXP, nsIWidget::SetWindowTransparency will
call SetWindowLong to put the window into LAYERED mode. Invalidation and
painting for these windows will have to be modified because layered windows
don't invalidate and repaint normally. When Gecko calls nsIWidget::Invalidate,
the Windows widget code will issue an internal paint event. When the event is
received we set up a 32-bit bitmap, create a compatible DC and pass that DC into
the view manager as the DC to paint into. We capture the alpha values sent by
the view manager, and then when the view manager has finished painting we copy
the alpha values into the alpha channel part of the bitmap. Then we call
UpdateLayeredWindow, passing in the bitmap.

I think implementing this on GTK should be somewhat simpler. We just respond to
nsIWidget::UpdateTransparentWindowAlpha by updating the mask bitmap for the
window. (I guess we'll have to keep a cached mask bitmap around.)
Attachment #109176 - Flags: superreview?(peterv)
Attached patch checkpoint (obsolete) — Splinter Review
So here's where I'm at with general support for "background-color:transparent"
on windows. It doesn't work yet because I haven't tested it, but it does build
:-). This should show you what I'm trying to do. I'll try to get it working
tomorrow.

The widget code is only for GTK, Win32 support would have to be added
separately (and I can't do that since I don't have Windows around).
Attached patch better patch (obsolete) — Splinter Review
This moves the setting of the transparent window flag from PaintBackground to
SyncFrameViewProperties. It's going to be important to get this flag set
outside the paint event. It's also lower overhead.
Attachment #109985 - Attachment is obsolete: true
By the way, regarding -moz-window-region vs background-color:transparent...

Suppose the author wants to tile a background pattern over a window and also
have a transparent region. Well, if the window has a fixed size, then with
background-color:transparent the author can make a background image of that size
by hand-tiling the desired background pattern and then making the right parts
transparent --- should be easy with the right image editing tools. With
-moz-window-region you can use CSS to tile the background pattern, but then you
have to figure out the -moz-window-region coordinates by hand.

On the other hand if the window doesn't have a fixed size then
-moz-window-region isn't really usable.

The real difference is that -moz-window-region can clip elements such as text or
form elements in arbitrary ways. Thus I think it would be great to retarget
Jan's patch to be an extension to the existing 'clip' property.
Comment on attachment 110007 [details] [diff] [review]
better patch

The GTK code needs to be worked on to make it handle window resizing properly.
Right now it'll probably crash. When the window is resized we should also
resize the mask bit array and fill the "new" areas of the window with 1s.
ok, let's reuse the CSS part of my patch in future implementation of |clip:
polygon(x1,y1, x2,y2, ...)|
Attached patch patch v3 (obsolete) — Splinter Review
I'm going away until Jan 13 so here's where I'm at. The patch includes a lot of
#ifdef DEBUG_roc that you may find useful, if not just remove it.

I'll attach my testcase in another attachment. With this testcase, I have the
following problems remaining:
-- I'm not setting the GTK mask on the correct GtkWidget, I think, because the
window becomes sort-of-transparent but doesn't really work properly (maybe this
is because the border decorations are still there?) Anyway, I think this is a
GTK API problem because the correct alpha values ARE being sent down by the
view manager and the generated mask bits look right.
-- After some number of invalidates, we crash or hang. I think something in
this patch is corrupting memory :-(. Sorry, I was not able to track it down.
(Maybe we need to release the rendering contexts in ~BlendingBuffers() before
we destroy their surfaces?)
-- I haven't had a chance to test ComputeAlphas24 or ComputeAlphas32.
-- Need to fix up the mask array when we resize the GTK window. This doesn't do
that and usually just crashes when you resize.

For Windows, one thing that might be fairly easy to do is to build a similar
bit mask array as for GTK and then turn that into an HRGN and use SetWindowRgn.
Turn the bit mask into an HRGN by trying to a set of rectangles that combined
give you the bit mask. Of course the cool thing to do would be to use
UpdateLayeredWindow, which would let you use all 8 bits of alpha sent down by
the view manager, but that's a heck of a lot of work I think.

Good luck!
Attachment #110007 - Attachment is obsolete: true
Attached file XUL testcase (obsolete) —
run dist/bin/mozilla -chrome <thisfile.xul>
Attached image screenshot
I've made considerable progress. Here's a screenshot. I'll attach the testcase
that generated the screenshot in a moment.
Attached file XUL testcase
UL testcase attached. Note "hidechrome", which I have revived support for, and
the "background:transparent".
Attachment #110066 - Attachment is obsolete: true
Awesome!
I can try your patch as soon as I land other stuff from my tree.
WOAH! Animated GIFs just worked, first time I tried them. You have to see this
to believe it.
Trust me, the octopus is animated on the desktop just like it is when you
display the GIF in your browser window!
Robert, you rock!
This will be killer feature of XUL
I'll attach my patch in a moment. It fixes all the issues I was stuck with
before. It's in pretty good shape actually, seems quite stable. It's not perfect
though; it doesn't really work when the XUL contains native child widgets (e.g.,
scrolling content). You probably don't need that right away, although it would
be super-cool to render Web pages directly onto the desktop. (But we are close
to being able to do that.)
Attached patch patch (obsolete) — Splinter Review
Promised patch. This only does GTK of course. Someone else will have to do the
Windows and Mac parts.

BTW I used valgrind to find the memory corruption I was getting earlier. Worked
great.
Robert, what about landing what we have now and add implementation for other
platforms later ?
Mozilla 1.3 freeze is so close and I'm not sure if I will be able to add well
tested windows implementaton before the freeze.
if we can get it reviewed, sure.
roc, we should change your nick to rock+moz because, really, really, really,
you ***ROCK*** !!! This is awesome.
Attached patch Revised patch: layout changes (obsolete) — Splinter Review
I've updated the patch with better comments (and also some changes to
nsContainerFrame that I forgot to include last time) and split it into three
parts: layout (to be reviewed by kin), views (to be reviewed by kmcclusk) and
widget+xpfe+gfx (to be reviewed by bryner).
Attachment #111904 - Attachment is obsolete: true
Attachment #111957 - Flags: superreview?(kin)
Attachment #111957 - Flags: review?(kin)
Attachment #111959 - Flags: superreview?(kin)
Attachment #111959 - Flags: review?(kmcclusk)
Attachment #111960 - Flags: superreview?(kin)
Attachment #111960 - Flags: review?(bryner)
If this bug is fixed will that make Mozilla skinable in the same way as Real Player?
Comment on attachment 111959 [details] [diff] [review]
view changes

r=kmcclusk@netscape.com
Attachment #111959 - Flags: review?(kmcclusk) → review+
Attachment #111960 - Attachment is obsolete: true
Attachment #111960 - Flags: superreview?(kin)
Attachment #111960 - Flags: review?(bryner)
Attached patch revised fix for gfx+widget+xpfe (obsolete) — Splinter Review
This version adds some extra checks to catch potential issues with, say,
someone trying to load transparent chrome into an embedded context. We should
not honour transparency in that case.
Attachment #112241 - Flags: superreview?(kin)
Attachment #112241 - Flags: review?(bryner)
Attached patch revised view patch (obsolete) — Splinter Review
The last view patch had some bad hunks that belonged to another patch. This one
doesn't.
Attachment #111959 - Attachment is obsolete: true
Comment on attachment 112243 [details] [diff] [review]
revised view patch

carrying forward r=, resetting kin sr request
Attachment #112243 - Flags: superreview?(kin)
Attachment #112243 - Flags: review+
Attachment #111959 - Flags: superreview?(kin)
Attachment #111959 - Flags: review+
Attachment #111957 - Flags: superreview?(kin)
Attachment #111957 - Flags: review?(kin)
Attached patch improved layout patch (obsolete) — Splinter Review
Here's a new layout patch. This gets rid of the XUL dependency.
Attachment #111957 - Attachment is obsolete: true
Attachment #112245 - Flags: superreview?(kin)
Attachment #112245 - Flags: review?(kin)
Comment on attachment 112241 [details] [diff] [review]
revised fix for gfx+widget+xpfe

I'm not all that familiar with the gfx stuff here, but I trust roc.  The widget
code looks good, except that it could use a comment about how ChangedMaskBits
and UpdateMaskBits work.  Have you tested Txul with this change?
Attachment #112241 - Flags: review?(bryner) → review+
I hate to do this this late in the game, but unless someone can prove to me why
this is actually a good idea and not just this weeks cool shiny object, I'm
going to pull out my module owner badge and block this from going in to the tree.
I should add that if this "cool new feature" in XUL didn't require the massive
amounts of new code that I wouldn't have as much of a problem with it, but as it
is, this would add a lot of extra code that can't be factored out if you don't
want to include XUL.
Jan wants it for some Netscape/AOL project, I assume. I thought other people
could use this too, for themes and XUL apps and mozdev hacks like the circular
pop-up menu for mouse gestures. (To get translucent popups working might require
a few more changes to layout, but not much.)

The only major code addition is in GFX. The view patch is almost entirely
refactoring that should be done anyway, and the layout patch is small.

We could shrink the GFX/GTK code in a few ways. One way would be to not support
changing the mask or resizing the window. Another option would be to leave the
GFX hooks as they are, put the GTK-specific support inside #if 0 (or remove it
entirely), and let other platform owners decide whether they want the support.
Another option would be to put the GTK support inside #ifdef INCLUDE_XUL or the
equivalent thereof.
Perhaps make it a build flag so that people who don't want this support can 
disable it.. then it could work in addition to disabling XUL.  If it is going 
to be done, I don't think you can really remove the resizing or mask changing...

If you add up the different platforms code to support this, we're looking at a 
lot of new code being added to an area that isn't very well owned.  Are you 
going to own this code and fix all the bugs in it?

Is there someone signed up to do the work on Windows and on Mac?
I'm happy to own the GTK code for this. I already own views and nsBlender (the
latter perhaps not officially, but in practice).

I think Jan is planning to do the Windows work. I don't know if anyone is going
to  sign up for other platforms. But they don't, they just don't get the feature
on those platforms, and the only impact is 200 lines in nsBlender that they
won't be using.

I don't think we support top-level HTML windows, so there is no point in
enabling this if XUL is disabled. I think we may as well just test for XUL.
Yeah, I plan to add platform code for windows and maybe for mac too.
ok, so can we figure out what code isn't needed if xul is disabled?  bryner has 
builds working again without XUL so this is pretty important.  If we get that 
and Jan is going to own the windows code, then I'm ok with it going in.
Attached patch gfx+widget+xpfe patch, revised (obsolete) — Splinter Review
OK, here are my INCLUDE_XULs to disable top-level window features if we're not
building with XUL.
Attachment #112241 - Attachment is obsolete: true
Attachment #112241 - Flags: superreview?(kin)
please use MOZ_XUL, not INCLUDE_XUL.  I'm changing all instances of INCLUDE_XUL
to MOZ_XUL in another patch.
Any idea when this may land?
Attachment #113864 - Flags: superreview?(kin)
Attachment #113864 - Flags: review?(pavlov)
Certainly not before 1.3final is branched. If you need this soon, consider
sending someone to pavlov and kin's offices to 'encourage' them to review the
patches.
Comment on attachment 113864 [details] [diff] [review]
replaced INCLUDE_XUL with MOZ_XUL

I don't understand why you removed this code..
http://lxr.mozilla.org/seamonkey/source/widget/src/gtk/nsWindow.cpp#872

Does the view manager do something similar now?  Even with the correct clipping
being set, this gave a very large performance increase on Unix when it was
added.	Why are we sending the bounding box at all if we're sending the region
in the nsPaintEvent?

Aside from that, the rest looks ok.
Sorry, that hunk is part of another patch and it slipped in by mistake. I'll
remove it and repost the patch.

FYI that hunk is part of the patch for bug 191474.
Comment on attachment 113864 [details] [diff] [review]
replaced INCLUDE_XUL with MOZ_XUL

ok, r=pavlov
Attachment #113864 - Flags: review?(pavlov) → review+
Target Milestone: mozilla1.3alpha → mozilla1.4alpha
There is a nice tutorial - Win32 Window Skinning
http://flipcode.com/articles/article_win32skins.shtml

Since the last patch differs from my last patch a lot, especially that we use
bitmaps instead of shapes (regions), we need to do it differently on windos.
::SetLayeredWindowAttributes() looks promising
There are two ways to go on Windows. You can either use SetWindowRgn or you can
use the Win2K/WinXP "layered window" APIs. The layered window APIs are probably
faster and they support translucency, but I suspect they'll be harder to
implement   since to get per-pixel alpha you have to use UpdateLayeredWindow,
which requires a change in the way we do painting for these windows. (You don't
do WM_PAINT/BeginPaint()/EndPaint() at all.) Also, layered windows won't work on
Win9x.

So I suggest that as a first step, you try using SetWindowRgn. If that doesn't
perform satisfactorily, or you want translucency to work on Win2K/WinXP, then
you can add code to use UpdateLayeredWindow when it's available.

To use SetWindowRgn you'll need algorithm which converts a bitmap into a region.
It's not that hard to do. There may be sample code for doing this somewhere on
the Internet already. You can borrow the GTK code to keep track of the complete
bitmap showing which pixels are transparent, and run your region conversion
algorithm whenever the bitmap changes.
Taking this to keep it on my radar
Assignee: varga → roc+moz
Status: ASSIGNED → NEW
Whiteboard: [adt2] → [adt2][fix]
Comment on attachment 112243 [details] [diff] [review]
revised view patch

I didn't mean to give myself the review for this. sorry...
Attachment #112243 - Flags: superreview?(kin)
Attachment #112243 - Flags: superreview?(bzbarsky)
Attachment #112243 - Flags: review?(kmcclusk)
Attachment #112243 - Flags: review+
Attachment #112245 - Flags: superreview?(kin)
Attachment #112245 - Flags: superreview?(bzbarsky)
Attachment #112245 - Flags: review?(kin)
Attachment #112245 - Flags: review?(bzbarsky)
Attachment #113864 - Flags: superreview?(kin) → superreview?(bzbarsky)
Comment on attachment 112243 [details] [diff] [review]
revised view patch

r=kmcclusk@netscape.com
Attachment #112243 - Flags: review?(kmcclusk) → review+
Comment on attachment 112243 [details] [diff] [review]
revised view patch

>Index: view/src/nsViewManager.cpp
>+  it's an alternative double-buffer buffer that starts of white instead of black, and

"starts off".

>+  PRBool transparentWindow = PR_FALSE;
>+  if (widget) {
>+    widget->GetWindowTransparency(transparentWindow);
>+    if (transparentWindow) {
>+      // for a translucent window, we'll pretend the whole area is
>+      // translucent.
>+      mTranslucentArea = aRect;

Hmm... could we decide whether it's transparency or translucency and be
consistent (just to reduce confusion)?

It looks like there may be more drawing surface memory churn since we no longer
use global drawing surfaces and instead create them every time RenderViews is
called....  Is that going to have any performance impact?  Or do rendering
contexts do decent caching of drawing surfaces?

>+void nsViewManager::RenderDisplayListElement(DisplayListElement2* element,
>+  nsIRenderingContext &aRC, BlendingBuffers* aBuffers)

This is really oddly indented... Just make it three lines and indent all params
equally?

>+BlendingBuffers::BlendingBuffers(nsIRenderingContext* aRC) {
>+  mCleanupContext = aRC;

Would a more descriptive name than "aRC" be in order for that argument?  Like
"aCleanupContext" or something?

>   // create a blender, if none exists already.
>   if (nsnull == mBlender) {
>     rv = nsComponentManager::CreateInstance(kBlenderCID, nsnull, NS_GET_IID(nsIBlender), (void **)&mBlender);

While you're here, 

  rv = CallCreateInstance(kBlenderCID, &mBlender);

For that matter, make mBlender an nsCOMPtr and

  mBlender = do_CreateInstance(kBlenderCID, &rv);

sr=bzbarsky with the nits picked and the question about memory churn addressed.
Attachment #112243 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 112245 [details] [diff] [review]
improved layout patch

>Index: layout/html/base/src/nsContainerFrame.cpp
>     if (nsnull == rootParent) {
>       viewHasTransparentContent = PR_FALSE;

if (!rootParent) ?

>+  // XXX we should also set widget transparency for XUL popups that ask for it

Wouldn't that just work if you skipped the "toplevel document" check and set
widget transparency for any widget whose view has no parent?

In any case, could you add a comment right before the

+      if (bg->mBackgroundFlags & NS_STYLE_BG_COLOR_TRANSPARENT) {

line saying that we need to set window transparency on toplevel windows with
transparent backgrounds?

>+  return canvasFrame != nsnull
>+      ? FindCanvasBackground(aPresContext, canvasFrame, aBackground)

return canvasFrame ? ... : ...;

should work fine, no?

>     if (nsnull == rootParent) {
>-      // Ensure that we always paint a color for the root (in case there's

if (!rootParent)

r+sr=bzbarsky with the nits picked.
Attachment #112245 - Flags: superreview?(bzbarsky)
Attachment #112245 - Flags: superreview+
Attachment #112245 - Flags: review?(bzbarsky)
Attachment #112245 - Flags: review+
Comment on attachment 113864 [details] [diff] [review]
replaced INCLUDE_XUL with MOZ_XUL

>Index: gfx/src/nsBlender.cpp

>+  NS_ASSERTION(PR_FALSE, "GetAlphas not implemented because XUL support not built");

NS_ERROR, please.

>+/**
>+ * Let A be the unknown source pixel's alpha value and let C be its (unknown) color.
>+ * Let S be the value painted onto black and T be the value painted onto white.
>+ * Then S = C*(A/255) and T = 255*(1 - A/255) + C*(A/255).
>+ * Therefore A = 255 - (T - S)
>+ * This is true no matter what color component we look at.
>+ */
>+static void ComputeAlphas32(PRInt32 aNumLines, PRInt32 aNumBytes,
>+                            PRUint8 *aS1Image, PRUint8 *aS2Image,
>+                            PRInt32 aSLSpan, PRUint8 *aAlphas)

Could we make the args match the comment a bit better?	In particular, is
aS1Image or aS2Image painted on white?

>+    PRUint8 *s1 = aS1Image + 1;
>+    PRUint8 *s2 = aS2Image + 1;

Hmm... Why the + 1?  Because we're looking at component #1 (as opposed to
component #0)?
If so, please move that comment up here....

>+static void ComputeAlphas24(PRInt32 aNumLines, PRInt32 aNumBytes,
>+                            PRUint8 *aS1Image, PRUint8 *aS2Image,
>+                            PRInt32 aSLSpan, PRUint8 *aAlphas)

This is identical to ComputeAlphas32 except for doing +=3 instead of +=4. 
Should we just have one function that takes the "3" or "4" as an argument?

The comments above apply here too, of course.

>+      PRUint32 pix1 = GREEN16(*s1) >> (8 - BLEND_GREEN_BITS);
>+      PRUint32 pix2 = GREEN16(*s2) >> (8 - BLEND_GREEN_BITS);

Hmm... Could you comment those two lines a little bit?	Whence comes the magic
number "8"?  Why are we doing that bitshift?

All of this is using aNumBytes to mean aBytesPerLine; should it be called that?

>+  nsIDrawingSurface* srcSurface = (nsIDrawingSurface *)aBlack;
>+  nsIDrawingSurface* secondSrcSurface = (nsIDrawingSurface *)aWhite;

This I do not like one bit... if this is legal, why are we bothering with
nsDrawingSurface at al instead of just using nsIDrawingSurface?

Also, why are we losing the descriptive black/white names and using srcSurface
and secondSrcSurface?

>+  rangeCheck(srcSurface, r.x, r.y, r.width, r.height);
>+  rangeCheck(secondSrcSurface, r.x, r.y, r.width, r.height);
>+
>+  result = srcSurface->Lock(aRect.x, aRect.y, aRect.width, aRect.height,
>+                            (void**)&srcBytes, &srcRowBytes, &srcSpan, 
NS_LOCK_SURFACE_READ_ONLY);

What's the point of doing the rangeCheck if we're just going to use aRect here
and below?

>+          ComputeAlphas(srcBytes, srcRowBytes, secondSrcBytes, srcSpan,
>+                        aRect.height, *aAlphas, aRect.width*aRect.height, depth);

Hmm... Could we make the argument orders for ComputeAlphas and ComputeAlphasN
(for various N) match as much as possible?

>Index: widget/public/nsIWidget.h

>+     * Set the transparency of the top-level window that contains this
>+     * widget. This can fail if the platform doesn't support
>+     * transparency.

So if I call this on an <iframe>s widget, the transparency of the browser
window that contains the <iframe> will be set?	If not, could we adjust this
comment accordingly?

>Index: widget/src/gtk/nsWindow.cpp

>+  NS_RELEASE(event.renderingContext);

File a bug to make nsPaintEvent have an nsCOMPtr member there, maybe?

>   if (!mUpdateArea->IsEmpty()) {
>-
>-    PRUint32 numRects;
>-    mUpdateArea->GetNumRects(&numRects);

This looks like sneak-over from your GTK over-painting patch....  Which parts
of this diff are actually relevant to this bug?

I'm not reading the guts of the bit-twiddling code for now, pending getting
that part cleared up....

>Index: widget/src/gtk/nsWindow.h

>-  PRBool	      mIMECallComposeStart;
>-  PRBool	      mIMECallComposeEnd;
>-  PRBool	      mIMEIsBeingActivate;
>+  PRPackedBool        mIMEEnable;
>+  PRPackedBool	      mIMECallComposeStart;
>+  PRPackedBool	      mIMECallComposeEnd;
>+  PRPackedBool	      mIMEIsBeingActivate;

Tabs?  Please fix?
Attachment #113864 - Flags: superreview?(bzbarsky) → superreview-
> Hmm... could we decide whether it's transparency or translucency and be
> consistent (just to reduce confusion)?

OK, I'll change all the new names to translucency since that's what we actually
support at the API level. (They're not synonyms; translucency means that you can
have pixels with nonintegral alpha values.)

> It looks like there may be more drawing surface memory churn since we no
> longer use global drawing surfaces and instead create them every time
> RenderViews is called....  Is that going to have any performance impact?  Or
> do rendering contexts do decent caching of drawing surfaces?

We already reallocate the drawing surface for double-buffering on every
RenderViews by calling GetBackbuffer, which caches on the platforms that need it
(Mac). Windows and GTK don't cache because caching is actually a performance
LOSE on those platforms (starves other apps of graphics memory). This patch
changes to always reallocate buffers for translucency, which will slow down Mac
translucency, but alleviate resource consumption on all platforms because we
usually don't need these buffers so keeping them around is wasting resources 90%
of the time.

In future GTK work (hopefully not too future) I want to move all the buffer
manipulation down into GFX where the Mac can manage its buffers however it likes.

> This is really oddly indented... Just make it three lines and indent all
> params equally?

OK

> Would a more descriptive name than "aRC" be in order for that argument?  Like
> "aCleanupContext" or something?

OK

> For that matter, make mBlender an nsCOMPtr and
>  mBlender = do_CreateInstance(kBlenderCID, &rv);

OK

> if (!rootParent)

OK, if you insist :-)

> >+  // XXX we should also set widget transparency for XUL popups that ask for
> > it
> Wouldn't that just work if you skipped the "toplevel document" check and set
> widget transparency for any widget whose view has no parent?

No, I think the view for a popup is parented normally in the view hierarchy.

> line saying that we need to set window transparency on toplevel windows with
> transparent backgrounds?

OK

> return canvasFrame ? ... : ...;
> should work fine, no?

Yeah. Maybe I'm too acclimated to Java, but I dislike the
treat-a-pointer-as-a-boolean style. But I'll do what I have to do to make you
happy :-).

More in the next comment...
> NS_ERROR, please.

OK

> Could we make the args match the comment a bit better?
> In particular, is aS1Image or aS2Image painted on white?

aS1Image. Sure.

> Hmm... Why the + 1?  Because we're looking at component #1 (as opposed to
> component #0)?
> If so, please move that comment up here....

Yep. OK.

> Should we just have one function that takes the "3" or "4" as an argument?

Yeah, sure.

> Hmm... Could you comment those two lines a little bit?
> Whence comes the magic number "8"?  Why are we doing that bitshift?

Sure. GREEN16 returns a value between 0 and 255 representing the green value of
the pixel. It only have BLEND_GREEN_BITS of precision (so the values are 0, 8,
16, ..., 248). The shift converts these to 0..31 by dropping the (8 -
BLEND_GREEN_BITS) low-order bits. We then scale the values up to the 0..255
range by multiplying by 255/31. If we just used the GREEN16 values directly in
the same equations that we use for the 24-bit case, we'd lose because (e.g.) a
completely transparent pixel would have GREEN16(pix1) = 0, GREEN16(pix2) = 248,
and the resulting alpha value would just be 248, but we need 255. So we need to
do this rescaling.

Hmm, now I see that this could be simplified to
      PRUint32 pix1 = GREEN16(*s1);
      PRUint32 pix2 = GREEN16(*s2);
      *aAlphas++ = (PRUint8)(255 - ((pix2 - pix1)*255)/(((1 << BLEND_GREEN_BITS)
- 1) << (8 - BLEND_GREEN_BITS)));
but that will still need a comment :-)

> All of this is using aNumBytes to mean aBytesPerLine; should it be called
> that?

Yes it should.

> This I do not like one bit... if this is legal, why are we bothering with
> nsDrawingSurface at al instead of just using nsIDrawingSurface?

Legacy code; we've been doing this on the Blend path for years. The whole
drawing surface API is broken. My plan is to first drive all uses of drawing
surfaces down into GFX, and then to changes uses of
nsDrawingSurface/nsIDrawingSurface to plain platform-specific APIs.

> Also, why are we losing the descriptive black/white names and using srcSurface
> and secondSrcSurface?

Legacy code copied from the Blend path. I'll fix that.

> What's the point of doing the rangeCheck if we're just going to use aRect here
> and below?

My mistake. I'll fix it.

> Hmm... Could we make the argument orders for ComputeAlphas and ComputeAlphasN
> (for various N) match as much as possible?

OK, I'll try.

> So if I call this on an <iframe>s widget, the transparency of the browser
> window that contains the <iframe> will be set?

Yeah. This is unfortunate but it avoids the need for layout to find the
nsIWidget for the top-level window, which is not trivial. I suppose I should
change this so that it only affects the current widget, and only if that widget
is a top-level nsIWidget (no parent); then layout will have to chase nsIWidget
parents to find the top. That's probably a nicer interface.

> File a bug to make nsPaintEvent have an nsCOMPtr member there, maybe?

OK

> This looks like sneak-over from your GTK over-painting patch....
> Which parts of this diff are actually relevant to this bug?

Oh dear, yes it is. Ignore the entire hunk at @@ -789,121 +795,82 @@.

> Tabs?  Please fix?

Legacy tabs :-). I will fix.
Attached patch complete patch (obsolete) — Splinter Review
Revised patch addressing all bz comments as I promised.

I changed the comments on nsIWidget::GetWindowTranslucency to indicate that
indeed, it really sets the state of whatever toplevel window contains the
indicated widget. It's just too hard for XP code in layout to find the top
level nsIWidget for the window. Chasing widget->GetParent() doesn't work; it
seems that the nsIWidget parenting chain isn't fully hooked up.
Attachment #109176 - Attachment is obsolete: true
Attachment #112243 - Attachment is obsolete: true
Attachment #112245 - Attachment is obsolete: true
Attachment #113864 - Attachment is obsolete: true
Comment on attachment 117409 [details] [diff] [review]
complete patch

Throwing back to bz for approval.
Attachment #117409 - Flags: superreview?(bzbarsky)
Attachment #117409 - Flags: review?(bzbarsky)
I'll try to get to this on Wed, but if I haven't by Friday then I won't be able
to until sometime in April....
Robert, I just noticed you added this to landing page.
I'm currently pretty busy with bookmarks branch. After that lands I'd like to
make  transparent chrome windows work on Windows (early 1.4 beta).
Comment on attachment 117409 [details] [diff] [review]
complete patch

>+   order. This array must be freed by the caller.

Uber-nit.  Make it clear that it must be deallocated with delete[].

>+  ResizeTransparencyBitmap(aWidth, aHeight);
>+
>   mBounds.width  = aWidth;
>   mBounds.height = aHeight;

Add a comment here that ResizeTransparencyBitmap needs the old bounds...

>+static PRBool ChangedMaskBits(gchar* aMaskBits, PRInt32 aMaskWidth, PRInt32 aMaskHeight,
>+                              const nsRect& aRect, PRUint8* aAlphas) {

Maybe MaskBitsHaveChanged ?

>+NS_IMETHODIMP nsWindow::UpdateTranslucentWindowAlpha(const nsRect& aRect, PRUint8* aAlphas) {
>+  if (mTransparencyBitmap == nsnull) {
>+    PRInt32 size = ((mBounds.width+7)/8)*mBounds.height;
>+    mTransparencyBitmap = new gchar[size];
>+    if (mTransparencyBitmap == nsnull)

if (!mTransparencyBitmap) in both places, please.

Looks good otherwise (though I have to trust pav on his review of the proper
use of some the X-guts...)
Attachment #117409 - Flags: superreview?(bzbarsky)
Attachment #117409 - Flags: superreview+
Attachment #117409 - Flags: review?(bzbarsky)
Attachment #117409 - Flags: review+
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Apparently this caused the regressions in bug 198946 and bug 198983. It will be
backed out in a moment. I need to figure out why these regressions happened when
my debug build works OK. It could be opt vs debug. It's not classic vs modern,
because bug 198946 happens with classic theme on my laptop.
reopening.  I backed out the fix from darin's machine.  due to regression
blockers today. 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch revised patchSplinter Review
Here's a new patch that fixes the issues found with the last patch. Here are
the deltas from the last patch:

* Added some safety assertions inside ComputeAlphas to alert if we're
overrunning the alphas buffer. (These don't fire in my tests, but in future it
would be helpful to know if they do.)
* I recently added an optimization to GTK painting in nsWindow.cpp that stops
painting if the window is obscured. This needs to be disabled for translucent
windows, because a translucent window can be obscured just because all its
pixels are currently transparent, but we still need to paint it in case the
pixel alphas change.
* Changed the translucent-window-setting logic in nsContainerFrame so that it
enables or disables the setting every time the root view is synced. (Before we
just enabled it the first time the document looked transparent; this caused
problems when the document goes through a transient state where it is
transparent but is "really" opaque in the steady state.) Also changed this
logic so that it always only applies to XUL documents, never HTML (hidden
about:blank window was going transparent). Also added checks to fix the print
crasher.
* Added a warning to nsViewManager whenever we paint a translucent window. This
ensures we'll know if we turn on translucency by accident. I verified that this
warning never fires during normal operation.
Attachment #117409 - Attachment is obsolete: true
Am i right in asuming this will be a big feture, espesially in the next Netscape
release?
Comment on attachment 119572 [details] [diff] [review]
revised patch

Reapplying for r/sr.

I think this should be landable in 1.4beta since the changes on normally
executed paths are fairly small.
Attachment #119572 - Flags: superreview?(bzbarsky)
Attachment #119572 - Flags: review?(bzbarsky)
Comment on attachment 119572 [details] [diff] [review]
revised patch

Looks fine.
Attachment #119572 - Flags: superreview?(bzbarsky)
Attachment #119572 - Flags: superreview+
Attachment #119572 - Flags: review?(bzbarsky)
Attachment #119572 - Flags: review+
Checked in again ... let's hope it sticks this time!
Status: REOPENED → RESOLVED
Closed: 21 years ago21 years ago
Resolution: --- → FIXED
Oh no.  I'm sad to say that I'm once again seeing some of the strange weirdness
that I was seeing with the last version of the patch.

Specifically, ^F to bring up the Find dialog brings up a titlebar and a
totally-transparent window the first time, works subsequent times (linux/gtkfe/x86).
strange. some dialogs work, others don't.

I'll see if I can get a fix for this today. If not, I'll back it out tonight.
Attached patch fix...Splinter Review
The problem is that we seem to occasionally paint a window when it's in a bad
state (no styles resolved). This is nondeterministic. The window element is
then transparent, which causes us to activate window translucency. And it seems
that once we've activated window translucency, GTK won't let us turn it off and
does strange things when we try.

On the other hand, we can't delay too long because if we set the window mask
for the first time after the window has been Shown, it won't take effect. (We
can change the mask though, as long as it was first set before the window was
Shown. Weird).

So the solution is to delay setting the window mask for the first time until
the window is shown.
Attachment #119640 - Flags: review+
Attachment #119640 - Flags: superreview+
Fix checked in. I hope this is it! No more transparent dialogs for me.

If this doesn't work, I'm backing the whole mess out and going to bed.
This check-in broke a couple of the ports tinderbox.
You declare "void ApplyTransparencyBitmap();" yet
you have it "return return NS_ERROR_FAILURE;" "if (!maskBitmap)"
and you don't check for any return code.

I am going to re-open this bug and just "return" if
the maskBitmap doesn't get created.  If this is "ok" 
then you can just close this bug again, otherwise, 
you can deal with it a different manner.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
ok, bryner actually fixed this... 
I am just gonna reset this to fixed...
Status: REOPENED → RESOLVED
Closed: 21 years ago21 years ago
Resolution: --- → FIXED
This check-in added 4k in codesize!
Is there anyway to optimize?

  Overall Change in Size
  	Total:	      +4364 (+7024/-2660)
  	Code:	      +4176 (+6832/-2656)
  	Data:	       +188 (+192/-4)
  
  libgklayout.so
  	Total:	      +2044 (+4556/-2512)
  	Code:	      +2048 (+4556/-2508)
  	Data:	         -4 (+0/-4)
  	      +2048 (+4556/-2508)	T (CODE)
  		      +2048 (+4556/-2508)	UNDEF:libgklayout.so:T
  			      +1280	nsViewManager::RenderDisplayListElement(DisplayListElement2 *,
nsIRenderingContext &, BlendingBuffers *)
  			      +1048	nsViewManager::RenderViews(nsView *, nsIRenderingContext &,
nsRegion const &, int &)
  			      +1040	nsViewManager::CreateBlendingBuffers(nsIRenderingContext *, int)
  			       +372	SyncFrameViewGeometryDependentProperties(nsIPresContext *,
nsIFrame *, nsStyleContext *, nsIView *, unsigned int)
  			       +224	BlendingBuffers::~BlendingBuffers(void)
  			       +224	ConvertNativeRegionToAppRegion(nsIRegion *, nsRegion *,
nsIDeviceContext *)
  			       +144	nsCSSRendering::PaintBackground(nsIPresContext *,
nsIRenderingContext &, nsIFrame *, nsRect const &, nsRect const &, nsStyleBorder
const &, nsStylePadding const &, int)
  			       +108	BlendingBuffers::BlendingBuffers(nsIRenderingContext *)
  			        +92	nsCSSRendering::FindBackground(nsIPresContext *, nsIFrame *,
nsStyleBackground const **, int *)
  			        +16	nsCOMPtr_base::nsCOMPtr_base(nsISupports *)
  			         +8	nsViewManager::nsViewManager(void)
  			         -4	atexit
  			        -24	global constructors keyed to
nsViewManager::DestroyZTreeNode(DisplayZTreeNode *)
  			        -56	__static_initialization_and_destruction_0
  			       -316	nsViewManager::~nsViewManager(void)
  			       -932	nsViewManager::CreateBlendingBuffers(nsIRenderingContext &)
  			      -1176	nsViewManager::RenderDisplayListElement(DisplayListElement2 *,
nsIRenderingContext &)
  	         -4 (+0/-4)	? (DATA)
  		         -4 (+0/-4)	UNDEF:libgklayout.so:?
  			         -4	__CTOR_LIST__
  
  libwidget_gtk.so
  	Total:	      +1568 (+1704/-136)
  	Code:	      +1376 (+1512/-136)
  	Data:	       +192 (+192/+0)
  	      +1376 (+1512/-136)	T (CODE)
  		      +1376 (+1512/-136)	UNDEF:libwidget_gtk.so:T
  			       +360	nsWindow::ResizeTransparencyBitmap(int, int)
  			       +312	nsWindow::UpdateTranslucentWindowAlpha(nsRect const &, unsigned
char *)
  			       +188	UpdateMaskBits(char *, int, int, nsRect const &, unsigned char *)
  			       +172	ChangedMaskBits(char *, int, int, nsRect const &, unsigned char *)
  			       +168	nsWindow::SetWindowTranslucency(int)
  			       +112	nsWindow::FindTopLevelWindow(void)
  			        +84	nsWindow::GetWindowTranslucency(int &)
  			        +36	nsWindow::~nsWindow(void)
  			        +20	nsBaseWidget::GetWindowTranslucency(int &)
  			        +20	nsWindow::DoPaint(nsIRegion *)
  			        +16	nsWindow::Resize(int, int, int)
  			        +12	nsBaseWidget::SetWindowTranslucency(int)
  			        +12	nsBaseWidget::UpdateTranslucentWindowAlpha(nsRect const &,
unsigned char *)
  			         -4	nsWindow::UpdateIdle(void *)
  			         -4	nsWindow::UnqueueDraw(void)
  			         -4	nsWindow::QueueDraw(void)
  			         -4	nsWindow::OnEnterNotifySignal(_GdkEventCrossing *)
  			         -4	nsWindow::NativeGrab(int)
  			         -4	nsWindow::Enable(int)
  			         -8	nsWindow::nsWindow(void)
  			         -8	nsWindow::IMEComposeStart(unsigned int)
  			         -8	nsWindow::IMEComposeEnd(unsigned int)
  			         -8	atexit
  			        -40	nsWindow::MakeFullScreen(int)
  			        -40	nsWindow::HideWindowChrome(int)
  	       +192 (+192/+0)	D (DATA)
  		       +192 (+192/+0)	UNDEF:libwidget_gtk.so:D
  			        +32	ChildWindow virtual table
  			        +32	nsBaseWidget virtual table
  			        +32	nsCheckButton virtual table
  			        +32	nsIWidget virtual table
  			        +32	nsScrollbar virtual table
  			        +32	nsWindow virtual table
  
  libgkgfx.so
  	Total:	       +752 (+764/-12)
  	Code:	       +752 (+764/-12)
  	Data:	         +0 (+0/+0)
  	       +752 (+764/-12)	T (CODE)
  		       +752 (+764/-12)	UNDEF:libgkgfx.so:T
  			       +372	nsBlender::GetAlphas(nsRect const &, void *, void *, unsigned
char **)
  			       +156	ComputeAlphas(int, int, int, unsigned char *, unsigned char *,
int, unsigned char *, unsigned int)
  			       +144	ComputeAlphas16(int, int, unsigned char *, unsigned char *,
int, unsigned char *, unsigned int)
  			        +92	ComputeAlphasByByte(int, int, int, unsigned char *, unsigned
char *, int, unsigned char *, unsigned int)
  			        -12	atexit
The layout changes, particularly the stuff in nsViewManager, were mostly code
reorganization that's necessary for other work (fixing our opacity model to
conform to CSS).

The GFX and widget changes are almost all #ifdef MOZ_XUL. Eventually embedding
should undefine MOZ_XUL and lose those changes.
Attached patch hidechrome patchSplinter Review
The hidechrome part of the fix got lost somehow.
Attachment #119871 - Flags: superreview?(bzbarsky)
Attachment #119871 - Flags: review?(roc+moz)
Attachment #119871 - Flags: superreview?(bzbarsky) → superreview+
Can someone point me to some info as to how to create and test examples of this?
 What do I need to do to get the examples to work?
Comment on attachment 119871 [details] [diff] [review]
hidechrome patch

just carrying forward r=bz from ages ago
Attachment #119871 - Flags: review?(roc+moz) → review+
andreww: try

mozilla -chrome http://bugzilla.mozilla.org/attachment.cgi?id=111901&action=view

Three caveats:
-- Only works on GTK right now.
-- Animation currently leaks X server memory. Not good.
-- There is a bug in nsBlender which means this will not work on displays which
use 24-bit color depth (crash). 16 or 32 bit depth is OK.
will look what it needs in BeOS port to be implemented
roc, want me to file a bug on the 24-bit nsBlender thing?
mmm, sure, that would be great, especially if you've still got the patch
Can anyone clarify a minor point.

Is the window corner option dependent on the window background attribute being
set  to transparent. Currently with (1.4b), if a I specify a default window
color, the corner is not removed (unlike box elements, etc)

<window style="background:yellow;-moz-border-radius-topleft:20px;" />

in other words, am I correct in thinking that the above styling will work as
expected, once the background:transparent logic is ported ( currently using win 98)
I can't get this to work either - Mozilla 1.4b or Firebird 0.6 - Win2000.
Brian, this has not yet been ported to Windows. See above:
> -- Only works on GTK right now.

I think I've fixed the leak I mentioned here:
> -- Animation currently leaks X server memory. Not good.

I'll fix this bug soonish:
> -- There is a bug in nsBlender which means this will not work on displays
> which use 24-bit color depth (crash). 16 or 32 bit depth is OK.

> am I correct in thinking that the above styling will work as expected

yes!
>>-- Only works on GTK right now.
I plan to work on the windows port in the near future.
This RFE has been marked as closed but is there still the intention to produce 
Windows support ?

I am really looking forward for this functionality.

I was also hoping to use the background transpancey to allow the creation of a 
titlebar widget which does not suffer from event capture problems outside of 
the visible window area.

Jan indicated in June it is planned but is there any timescale and what 
versions of Windows ?

(In reply to comment #114)
> This RFE has been marked as closed but is there still the intention to produce 
> Windows support ?
> 

We should probably file a new bug for windows port.

> Jan indicated in June it is planned but is there any timescale and what 
> versions of Windows ?
> 

Yeah, I was supposed to work on that, but the situation changed suddenly in July :(
If I only had time to work on this, sorry ...
Does the -chrome command line argument works in recent Mozilla trunk builds?
I've created alpha transparency code as a separate Win32 test application and
now I want to port it to Win32 nsWindow.cpp, but I can't find a way how to test it. 
When I pass test99.xul as a param the empty Mozilla window is opened with
standard chrome. When I open this xul in new window SetWindowTranslucency () is
called with false param. nsXULWindow::LoadChromeHidingFromXUL () does not see
"hidechrome" argument either. Seems that -chrome arg is just ignored.
*** Bug 40795 has been marked as a duplicate of this bug. ***
So where's the new bug for the Windows version?

Dainis, how is it going?  Can you file the bug?  Thanks,

/be
Opened followup bug 252067 to implement translucent windows for Win32.
This excellent work has some nits needing shakeout. See dependancies.

- N.
Status: RESOLVED → REOPENED
Depends on: 264707
Resolution: FIXED → ---
Nigel, this bug remains fixed. Regressions can stay in their own bugs. Thanks.
Status: REOPENED → RESOLVED
Closed: 21 years ago20 years ago
Resolution: --- → FIXED
Bug 264708 adds 1-bit transparency support for Win9x and Win NT.
Opened bug 307204 to add support for translucent windows in Mac
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: