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)

(deleted), application/zip
Details
(deleted), application/pdf
Details
(deleted), image/png
Details
(deleted), application/vnd.mozilla.xul+xml
Details
(deleted), image/gif
Details
(deleted), application/vnd.mozilla.xul+xml
Details
(deleted), image/png
Details
(deleted), patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
(deleted), patch
pavlov
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
(deleted), 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) (deleted) — Splinter Review
initial patch
Attached file testcase (obsolete) (deleted) —
testcase
Summary: support transparent non-rect chrome windows → support transparent non-rectangular chrome windows
Attached patch new patch (obsolete) (deleted) — Splinter Review
this one implements support for non-cotiguous regions too
Attachment #108282 - Attachment is obsolete: true
Attached file new testcase (deleted) —
Attachment #108283 - Attachment is obsolete: true
Joe, could you r/sr ?
Attachment #108992 - Flags: review?(glazman)
Attached patch better patch (obsolete) (deleted) — 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) (deleted) — 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) (deleted) — 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) (deleted) — 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) (deleted) — 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) (deleted) —
run dist/bin/mozilla -chrome <thisfile.xul>
Attached image screenshot (deleted) —
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 (deleted) —
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.
Attached image screenshot of the animated testcase (deleted) —
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) (deleted) — 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) (deleted) — 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
Attached patch view changes (obsolete) (deleted) — Splinter Review
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?
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) (deleted) — 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) (deleted) — 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) (deleted) — 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) (deleted) — 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.
Attached patch replaced INCLUDE_XUL with MOZ_XUL (obsolete) (deleted) — Splinter Review
Attachment #113367 - Attachment is obsolete: true
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)
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) (deleted) — 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: 22 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 patch (deleted) — Splinter 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: 22 years ago22 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... (deleted) — 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: 22 years ago22 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 patch (deleted) — Splinter 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: 22 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: