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)

x86
Windows Vista
defect
Not set
minor

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta1+

People

(Reporter: robarnold, Assigned: robarnold)

References

Details

(Keywords: perf)

Attachments

(2 files, 3 obsolete files)

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.
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
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.
can you attach a testcase?
Severity: normal → minor
Keywords: testcase-wanted
blocking2.0: --- → ?
Blocks: 544820
Blocks: 546259
No longer blocks: 544820
Depends on: layers
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.
Blocks: 546831
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)
Attached file Testcase
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.
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)
Keywords: testcase-wanted
+  void ToOutsidePixels (nsIntRegion &aRegion, nscoord aAppUnitsPerPixel) const;

Why can't we just return aRegion directly? If you do that, sr=me
(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.
(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)
Oops, forgot to qrefresh.
Attachment #433231 - Attachment is obsolete: true
Attachment #433232 - Flags: review?(jmathies)
Attachment #433231 - Flags: review?(jmathies)
Attachment #433232 - Flags: review?(jmathies) → review+
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/20f6578db03c
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Depends on: 554770
Depends on: 554959
blocking2.0: ? → beta1+
You need to log in before you can comment on or make changes to this bug.