Closed
Bug 450767
Opened 16 years ago
Closed 14 years ago
Don't necessarily apply Aero Glass to the entire window
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta1+ |
People
(Reporter: robarnold, Assigned: robarnold)
References
Details
(Keywords: perf)
Attachments
(2 files, 3 obsolete files)
631 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
14.67 KB,
patch
|
jimm
:
review+
|
Details | Diff | Splinter Review |
This can create a performance problem for large windows and it also removes the small window border drawn by the DWM. It would be nice to find a way to easily detect glass regions that are just on the edge of the window, and extend those margins inward only as far as necessary.
Comment 1•16 years ago
|
||
what are the performance implications? I run 30+ windows, 300+tabs and when I disabled aero in windows I am not able to detect any difference
Keywords: perf
Assignee | ||
Comment 2•16 years ago
|
||
The performance hit comes when the entire window is rendered as a glass. For most applications, only the title bar is (except when maximized). I had a test program where I could resize a glass panel and the performance hit was very noticeable when I made it full screen. For this reason, it's not feasible to enable an IE-like interface for the main browser window, or for any non-fixed size window for that matter.
Comment 3•15 years ago
|
||
can you attach a testcase?
Severity: normal → minor
Keywords: testcase-wanted
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 4•14 years ago
|
||
From IRC via roc, our strategy here is "hidechrome || empty opaque region -> don't use insets, otherwise set insets based on the largest rect in the opaque region" which seems feasible to check at paint time.
Assignee | ||
Comment 5•14 years ago
|
||
This patch adds a new method SetPossiblyTransparentRegion to nsIWidget with a default no-op implementation. For the win32 widget backend, we use the given region to compute the opaquely painted region. If that region is empty or the window has no caption (indicative of hidechrome), we set the glass margins to -1 to indicate the entire window should have glass. Otherwise we find the largest known opaque rectangle and use that as the sole non-glass region.
Attachment #432973 -
Flags: superreview?(roc)
Attachment #432973 -
Flags: review?(jmathies)
Assignee | ||
Comment 6•14 years ago
|
||
Here's a testcase that would be rather painful to move at large window sizes w/o this patch.
You should make SetPossiblyTransparentRegion take an nsIntRegion and do the conversion in layout. All the other widget APIs take device units. Otherwise looks pretty good.
Assignee | ||
Comment 8•14 years ago
|
||
This fixes the testcase when forcing a partial redraw. Also adds nsRegion::ToOutsidePixels.
Attachment #432973 -
Attachment is obsolete: true
Attachment #432998 -
Flags: superreview?(roc)
Attachment #432998 -
Flags: review?(jmathies)
Attachment #432973 -
Flags: superreview?(roc)
Attachment #432973 -
Flags: review?(jmathies)
Assignee | ||
Updated•14 years ago
|
Keywords: testcase-wanted
+ void ToOutsidePixels (nsIntRegion &aRegion, nscoord aAppUnitsPerPixel) const; Why can't we just return aRegion directly? If you do that, sr=me
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9) > + void ToOutsidePixels (nsIntRegion &aRegion, nscoord aAppUnitsPerPixel) > const; > > Why can't we just return aRegion directly? If you do that, sr=me We talked about this on IRC: [19:30:50] robarnold: isn't there some non-trivial overhead to the copy constructor of nsIntRegion? [19:31:05] robarnold: i.e. shouldn't I pass it by reference? [19:31:35] roc: yes you should At least, that's what I meant when I asked the question, but I think the answer means passing by reference here is the right thing to do still for performance reasons.
Status: NEW → ASSIGNED
If you do nsIntRegion ToOutsidePixels(...) { nsIntRegion result; ... return result; } then the named return value optimization kicks in and no copy occurs.
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #11) > If you do > nsIntRegion ToOutsidePixels(...) > { > nsIntRegion result; > ... > return result; > } > > then the named return value optimization kicks in and no copy occurs. I did not know that msvc and gcc could legally do this. Neat!
Attachment #432998 -
Attachment is obsolete: true
Attachment #433231 -
Flags: review?(jmathies)
Attachment #432998 -
Flags: superreview?(roc)
Attachment #432998 -
Flags: review?(jmathies)
Assignee | ||
Comment 13•14 years ago
|
||
Oops, forgot to qrefresh.
Attachment #433231 -
Attachment is obsolete: true
Attachment #433232 -
Flags: review?(jmathies)
Attachment #433231 -
Flags: review?(jmathies)
Updated•14 years ago
|
Attachment #433232 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 14•14 years ago
|
||
Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/20f6578db03c
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
blocking2.0: ? → beta1+
You need to log in
before you can comment on or make changes to this bug.
Description
•