Extend nsScreenManagerGonk to support multiple screens

RESOLVED FIXED in Firefox 40

Status

Firefox OS
General
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: shelly, Assigned: hchang)

Tracking

(Blocks: 1 bug)

unspecified
2.2 S12 (15may)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 9 obsolete attachments)

(Reporter)

Description

3 years ago
In order to support multi-screen on b2g, nsScreenGonk and nsScreenManagerGonk should extend itself of returning not only the primary screen.
In current architecture, there are most three screens, DISPLAY_PREIMARY, DISPLAY_VIRTUAL and DISPLAY_EXTERNAL for each.
So in this patch, I use mDisplayType
Assignee: nobody → kechen
Created attachment 8575027 [details] [diff] [review]
[WIP]Support ScreenManagerGonk and ScreenGonk to multi screen

Comment 1 is not complete, please ignore that. 

In current architecture, there are three display types, DISPLAY_PREIMARY, DISPLAY_VIRTUAL and DISPLAY_EXTERNAL, and most one screens for each type.
So in this patch, I use the display types as the screen id. In this way we don't need to add additional function in nsIScreen. We don't consider the rotation and ScreenForRect condition in current phase.
Attachment #8575027 - Attachment description: [WIP]0001-Support-ScreenManagerGonk-and-ScreenGonk-to-multi-sc.patch → [WIP]Support ScreenManagerGonk and ScreenGonk to multi screen
Created attachment 8577823 [details] [diff] [review]
Support ScreenManagerGonk and ScreenGonk-to multi-screen

Hi Shelly can you help and give me some advices to my modification?
Thank you.
Attachment #8575027 - Attachment is obsolete: true
Flags: needinfo?(slin)
(Reporter)

Comment 4

3 years ago
Looks good overall, thanks Kevin :)

I don't see any big problem here, but since the basic structure of nsWindow is not solid yet, let's leave it until then.
Flags: needinfo?(slin)
(Reporter)

Updated

3 years ago
No longer depends on: 1138287
(Assignee)

Updated

3 years ago
Assignee: kechen → hchang
(Assignee)

Comment 5

3 years ago
Created attachment 8599150 [details] [diff] [review]
Bug1138288.patch
Attachment #8577823 - Attachment is obsolete: true
(Assignee)

Comment 6

3 years ago
Created attachment 8599151 [details] [diff] [review]
Bug1138288.patch
(Assignee)

Updated

3 years ago
Attachment #8599150 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Blocks: 1138287
(Assignee)

Comment 7

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7fcdb094c2da
(Assignee)

Updated

3 years ago
Depends on: 1138290
(Assignee)

Comment 8

3 years ago
Created attachment 8599260 [details] [diff] [review]
Bug1138288.patch

Hi mwu,

This is the formal patch based on m-c and is simplified from attachment 8598468 [details] [diff] [review]. 
My plan is to land this patch first as the basis of further work in Bug 1138287. 
You can find the real world use case in attachment 8597207 [details].

Following is the summary of this patch:

1) The basic idea is to extend nsScreenManagerGonk to make it capable of
   adding/removing screens. By now we can only add primary type screen.
   But Bug 1138287 will be addressing the use of the extension.

2) nsWindow creation and nsScreenGonk creation is independent. The only 
   requirement is nsWindow must be opened in a created nsScreenGonk.

3) nsWindow originally grabs screen information from GonkDisplay.
   This patch gets rid of this dependency. nsWindow will be associated
   with a nsScreenGonk and obtain the required information like bound/native window
   from the screen.

4) nsScreenManagerGonk maintains a nsScreenGonk list rather than mOneScreen. 
   All the ScreenForXxx operation should loop throught the list and find the 
   matched one. The only exception is ScreenForRect since we don't know how to 
   handle this case.

5) We use "display type" to add/remove display. Since one display type may have
   multiple instances in the future, there is a conversion from display type
   to screen id. Right now it's nsScreenManagerGonk::GetIdFromType.


You can also find the enhancement of this patch if we have the enhanced GonkDisplay,
which can be requested to "AddDisplay" and returns the screen id in Bug 1138287.
This enhancement will be addressed in that bug.
Attachment #8599151 - Attachment is obsolete: true
Attachment #8599260 - Flags: review?(mwu)

Comment 9

3 years ago
Comment on attachment 8599260 [details] [diff] [review]
Bug1138288.patch

Review of attachment 8599260 [details] [diff] [review]:
-----------------------------------------------------------------

Bug 1157874 will be landing soon - can you rebase on that?
Attachment #8599260 - Flags: review?(mwu)

Updated

3 years ago
Depends on: 1157874
It might be better that nsScreenGonk creation is independent from widget(nsWindow) creation.
(Assignee)

Comment 11

3 years ago
(In reply to Michael Wu [:mwu] from comment #9)
> Comment on attachment 8599260 [details] [diff] [review]
> Bug1138288.patch
> 
> Review of attachment 8599260 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Bug 1157874 will be landing soon - can you rebase on that?

Sure! I will do that. Thanks!
(Assignee)

Comment 12

3 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #10)
> It might be better that nsScreenGonk creation is independent from
> widget(nsWindow) creation.

Hi Sotaro,

That's my intension! (See (2) in comment 8). The only exception of current patch 
is primary screen. I don't mind find a proper place to put "Add primary screen"
prior to opening the top level window if there is.

Btw, what's your argument on "maintaining a nsIScreen rather than the id"?
I am fine to both and I only worry about the if there would be any circular
reference issue.

Thanks!
(In reply to Henry Chang [:henry] from comment #12)
> (In reply to Sotaro Ikeda [:sotaro] from comment #10)
> > It might be better that nsScreenGonk creation is independent from
> > widget(nsWindow) creation.
> 
> Hi Sotaro,
> 
> That's my intension! (See (2) in comment 8). The only exception of current
> patch 
> is primary screen. I don't mind find a proper place to put "Add primary
> screen"
> prior to opening the top level window if there is.
> 
> Btw, what's your argument on "maintaining a nsIScreen rather than the id"?

nsWindow does not need to know screenId except to find out nsIScreen. Therefore basically no need to hold screen id. By holding screenId, nsWindow does not need to get nsIScreen each time usage. And it could extend the lifetime of nsIScreen even when the nsIScreen is removed from nsScreenManagerGonk.

> I am fine to both and I only worry about the if there would be any circular
> reference issue.

As a safe guard, it is safer to remove circular reference. Though, the circular reference does not cause a problem actually, because nsWindow::Destroy() is explicitly called during the widget destruction, that break the circular reference. nsScreenGonk is lower layer service than nsWindow. Therefore, it seems better that nsScreenGonk holds
> 
> nsWindow does not need to know screenId except to find out nsIScreen.
> Therefore basically no need to hold screen id. By holding screenId, nsWindow
> does not need to get nsIScreen each time usage. And it could extend the
> lifetime of nsIScreen even when the nsIScreen is removed from
> nsScreenManagerGonk.

It could make the screen destroy handling simpler.
(Assignee)

Comment 15

3 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #13)
> (In reply to Henry Chang [:henry] from comment #12)
> > (In reply to Sotaro Ikeda [:sotaro] from comment #10)
> > > It might be better that nsScreenGonk creation is independent from
> > > widget(nsWindow) creation.
> > 
> > Hi Sotaro,
> > 
> > That's my intension! (See (2) in comment 8). The only exception of current
> > patch 
> > is primary screen. I don't mind find a proper place to put "Add primary
> > screen"
> > prior to opening the top level window if there is.
> > 
> > Btw, what's your argument on "maintaining a nsIScreen rather than the id"?
> 
> nsWindow does not need to know screenId except to find out nsIScreen.
> Therefore basically no need to hold screen id. By holding screenId, nsWindow
> does not need to get nsIScreen each time usage. And it could extend the
> lifetime of nsIScreen even when the nsIScreen is removed from
> nsScreenManagerGonk.
> 
> > I am fine to both and I only worry about the if there would be any circular
> > reference issue.
> 
> As a safe guard, it is safer to remove circular reference. Though, the
> circular reference does not cause a problem actually, because
> nsWindow::Destroy() is explicitly called during the widget destruction, that
> break the circular reference. nsScreenGonk is lower layer service than
> nsWindow. Therefore, it seems better that nsScreenGonk holds

You are right! Considering the case of removing screens, nsWindow
needs to hold a strong nsIScreen reference. Thanks for reminding me!
(Assignee)

Comment 16

3 years ago
Created attachment 8599803 [details] [diff] [review]
Bug1138288.patch (rebased)
Attachment #8599260 - Attachment is obsolete: true
Attachment #8599803 - Flags: review?(mwu)
Comment on attachment 8599803 [details] [diff] [review]
Bug1138288.patch (rebased)

Review of attachment 8599803 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gonk/nsScreenManagerGonk.h
@@ +68,5 @@
>  protected:
> +    uint32_t mId;
> +    int32_t mPixelDepth;
> +    int32_t mColorDepth;
> +    void* mNativeWindow;

ANativeWindow is reference counted object. It is normally held like "sp<ANativeWindow> mNativeWindow;". Why does it hold as void pointer? Is there a no risk of this pointer becomes dangling pointer?
(In reply to Sotaro Ikeda [:sotaro] from comment #17)
> ANativeWindow is reference counted object. It is normally held like
> "sp<ANativeWindow> mNativeWindow;".

In gecko it becomes "android::sp<ANativeWindow>".
(Assignee)

Comment 19

3 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #18)
> (In reply to Sotaro Ikeda [:sotaro] from comment #17)
> > ANativeWindow is reference counted object. It is normally held like
> > "sp<ANativeWindow> mNativeWindow;".
> 
> In gecko it becomes "android::sp<ANativeWindow>".

Makes sense. Thanks for this review!

Comment 20

3 years ago
Comment on attachment 8599803 [details] [diff] [review]
Bug1138288.patch (rebased)

Review of attachment 8599803 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gonk/nsScreenManagerGonk.h
@@ +110,3 @@
>  
>      bool mInitialized;
> +    nsTArray<nsCOMPtr<nsScreenGonk>> mScreens;

use nsRefPtr for concrete classes

::: widget/gonk/nsWindow.cpp
@@ +889,5 @@
>  
>  // nsScreenGonk.cpp
>  
> +nsScreenGonk::nsScreenGonk(uint32_t aId,
> +                           const ScreenConfiguration& aConfig,

Using ScreenConfiguration here doesn't seem to help since the data needs to be packed into a ScreenConfiguration first.

@@ +892,5 @@
> +nsScreenGonk::nsScreenGonk(uint32_t aId,
> +                           const ScreenConfiguration& aConfig,
> +                           uint32_t aPhysicalRotation,
> +                           float aDpi,
> +                           void* aNativeWindow)

This is a ANativeWindow*. You can move the querying for width/height to the constructor here and reduce the number of args. Also, the format can be derived here by querying for NATIVE_WINDOW_FORMAT.

@@ +927,5 @@
> +nsScreenGonk::GetId()
> +{
> +    uint32_t id;
> +    GetId(&id);
> +    return id;

return mId;

@@ +948,5 @@
> +nsScreenGonk::GetRect()
> +{
> +    nsIntRect rect;
> +    GetRect(&rect.x, &rect.y, &rect.width, &rect.height);
> +    return rect;

return mScreenBounds;

@@ +975,5 @@
>  nsScreenGonk::GetPixelDepth(int32_t *aPixelDepth)
>  {
>      // XXX: this should actually return 32 when we're using 24-bit
>      // color, because we use RGBX.
> +    *aPixelDepth = mPixelDepth;

Let's just use mColorDepth for everything for now.

@@ +999,5 @@
> +{
> +    return mDpi;
> +}
> +
> +void*

Also an ANativeWindow*

@@ +1165,5 @@
>  
>  NS_IMETHODIMP
>  nsScreenManagerGonk::GetPrimaryScreen(nsIScreen **outScreen)
>  {
> +    for (size_t i = 0; i < mScreens.Length(); i++) {

Just return mScreens[0].

@@ +1259,5 @@
> +    }
> +
> +    char propValue[PROPERTY_VALUE_MAX];
> +    property_get("ro.sf.hwrotation", propValue, "0");
> +    uint32_t physicalRotation = atoi(propValue) / 90;

AFAIK, this rotation only applies to the primary display.
Attachment #8599803 - Flags: review?(mwu)
(Assignee)

Comment 21

3 years ago
Thanks for the review, mwu!

Please see my inline reply to some of your comments:

(In reply to Michael Wu [:mwu] from comment #20)
> Comment on attachment 8599803 [details] [diff] [review]
> Bug1138288.patch (rebased)
> 
> ::: widget/gonk/nsWindow.cpp
> @@ +889,5 @@
> >  
> >  // nsScreenGonk.cpp
> >  
> > +nsScreenGonk::nsScreenGonk(uint32_t aId,
> > +                           const ScreenConfiguration& aConfig,
> 
> Using ScreenConfiguration here doesn't seem to help since the data needs to
> be packed into a ScreenConfiguration first.
> 
> @@ +892,5 @@
> > +nsScreenGonk::nsScreenGonk(uint32_t aId,
> > +                           const ScreenConfiguration& aConfig,
> > +                           uint32_t aPhysicalRotation,
> > +                           float aDpi,
> > +                           void* aNativeWindow)
> 
> This is a ANativeWindow*. You can move the querying for width/height to the
> constructor here and reduce the number of args. Also, the format can be
> derived here by querying for NATIVE_WINDOW_FORMAT.
> 

My design guideline of nsScreenGonk/nsScreenManagerGonk is to let 
nsScreenManagerGonk be a screen factory. All the screen information
is queried and collected by nsScreenManagerGonk. nsScreenGonk
would just act like a plain plain object. This design guideline results in

1) We have to construct nsScreenGonk with all screen configuration through
   either a list of parameters or a parameter object. I was creating yet another
   ScreenConfiguration-like object but changed to reuse hal::ScreenConfiguration
   in the current patch. Now that there are 5 arguments, maybe I should use
   a parameter object again.

2) Any assumption like "pixel depth is equivalent to color depth" would
   be made in nsScreenManagerGonk. nsScreenGonk assumes nothing.

> @@ +927,5 @@
> > +nsScreenGonk::GetId()
> > +{
> > +    uint32_t id;
> > +    GetId(&id);
> > +    return id;
> 
> return mId;
> 
> @@ +948,5 @@
> > +nsScreenGonk::GetRect()
> > +{
> > +    nsIntRect rect;
> > +    GetRect(&rect.x, &rect.y, &rect.width, &rect.height);
> > +    return rect;
> 
> return mScreenBounds;
> 

I know there introduces some overhead to these getters. However, I think
it's better to have a single entry for these getters. For example, if
I rename mScreenBounds to mBounds, I only need to change one function 
but not two.

> @@ +975,5 @@
> >  nsScreenGonk::GetPixelDepth(int32_t *aPixelDepth)
> >  {
> >      // XXX: this should actually return 32 when we're using 24-bit
> >      // color, because we use RGBX.
> > +    *aPixelDepth = mPixelDepth;
> 
> Let's just use mColorDepth for everything for now.
> 
> @@ +1165,5 @@
> >  
> >  NS_IMETHODIMP
> >  nsScreenManagerGonk::GetPrimaryScreen(nsIScreen **outScreen)
> >  {
> > +    for (size_t i = 0; i < mScreens.Length(); i++) {
> 
> Just return mScreens[0].
> 

Since I am not making the assumption that the primary screen would
be always added firstly, I loop through the entire screen list to
make sure I get the primary one. If it could be a performance issue,
maybe I can use an additional member variable to point to the primary
screen. (However, I won't stick to not returning mScreens[0] :p )
Flags: needinfo?(mwu)

Comment 22

3 years ago
(In reply to Henry Chang [:henry] from comment #21)
> (In reply to Michael Wu [:mwu] from comment #20)
> > Comment on attachment 8599803 [details] [diff] [review]
> > Bug1138288.patch (rebased)
> > 
> > ::: widget/gonk/nsWindow.cpp
> > @@ +889,5 @@
> > >  
> > >  // nsScreenGonk.cpp
> > >  
> > > +nsScreenGonk::nsScreenGonk(uint32_t aId,
> > > +                           const ScreenConfiguration& aConfig,
> > 
> > Using ScreenConfiguration here doesn't seem to help since the data needs to
> > be packed into a ScreenConfiguration first.
> > 
> > @@ +892,5 @@
> > > +nsScreenGonk::nsScreenGonk(uint32_t aId,
> > > +                           const ScreenConfiguration& aConfig,
> > > +                           uint32_t aPhysicalRotation,
> > > +                           float aDpi,
> > > +                           void* aNativeWindow)
> > 
> > This is a ANativeWindow*. You can move the querying for width/height to the
> > constructor here and reduce the number of args. Also, the format can be
> > derived here by querying for NATIVE_WINDOW_FORMAT.
> > 
> 
> My design guideline of nsScreenGonk/nsScreenManagerGonk is to let 
> nsScreenManagerGonk be a screen factory. All the screen information
> is queried and collected by nsScreenManagerGonk. nsScreenGonk
> would just act like a plain plain object. This design guideline results in
> 
> 1) We have to construct nsScreenGonk with all screen configuration through
>    either a list of parameters or a parameter object. I was creating yet
> another
>    ScreenConfiguration-like object but changed to reuse
> hal::ScreenConfiguration
>    in the current patch. Now that there are 5 arguments, maybe I should use
>    a parameter object again.
> 
> 2) Any assumption like "pixel depth is equivalent to color depth" would
>    be made in nsScreenManagerGonk. nsScreenGonk assumes nothing.
> 

Ok, but I don't see the advantage of having all logic in nsScreenManagerGonk. It's just making the construction of nsScreenGonk more awkward. Move things to nsScreenGonk and simplify the construction of nsScreenGonk.

> > @@ +927,5 @@
> > > +nsScreenGonk::GetId()
> > > +{
> > > +    uint32_t id;
> > > +    GetId(&id);
> > > +    return id;
> > 
> > return mId;
> > 
> > @@ +948,5 @@
> > > +nsScreenGonk::GetRect()
> > > +{
> > > +    nsIntRect rect;
> > > +    GetRect(&rect.x, &rect.y, &rect.width, &rect.height);
> > > +    return rect;
> > 
> > return mScreenBounds;
> > 
> 
> I know there introduces some overhead to these getters. However, I think
> it's better to have a single entry for these getters. For example, if
> I rename mScreenBounds to mBounds, I only need to change one function 
> but not two.
> 

The overhead is not the concern. It's probably going to be optimized away. I only care about the number of lines of code. If getting rid of these getters entirely would reduce the number of lines, then do that. That would likely reduce the number of lines. I don't want any helper functions that add more lines of code overall.

> > @@ +975,5 @@
> > >  nsScreenGonk::GetPixelDepth(int32_t *aPixelDepth)
> > >  {
> > >      // XXX: this should actually return 32 when we're using 24-bit
> > >      // color, because we use RGBX.
> > > +    *aPixelDepth = mPixelDepth;
> > 
> > Let's just use mColorDepth for everything for now.
> > 
> > @@ +1165,5 @@
> > >  
> > >  NS_IMETHODIMP
> > >  nsScreenManagerGonk::GetPrimaryScreen(nsIScreen **outScreen)
> > >  {
> > > +    for (size_t i = 0; i < mScreens.Length(); i++) {
> > 
> > Just return mScreens[0].
> > 
> 
> Since I am not making the assumption that the primary screen would
> be always added firstly, I loop through the entire screen list to
> make sure I get the primary one. If it could be a performance issue,
> maybe I can use an additional member variable to point to the primary
> screen. (However, I won't stick to not returning mScreens[0] :p )

Why would the primary screen not get added first?

Don't design for cases that don't exist.
Flags: needinfo?(mwu)
(Assignee)

Comment 23

3 years ago
Created attachment 8601361 [details] [diff] [review]
Bug1138288 v2
Attachment #8599803 - Attachment is obsolete: true
(Assignee)

Comment 24

3 years ago
Created attachment 8601415 [details] [diff] [review]
Bug1138288 v2
Attachment #8601361 - Attachment is obsolete: true
(Assignee)

Comment 25

3 years ago
Created attachment 8601422 [details] [diff] [review]
Bug1138288 v2
Attachment #8601415 - Attachment is obsolete: true
(Assignee)

Comment 26

3 years ago
Comment on attachment 8601422 [details] [diff] [review]
Bug1138288 v2

Hi mwu,

I've addressed your comments in the patch v2. Could you please have it
a look again? Thanks!
Attachment #8601422 - Flags: review?(mwu)

Comment 27

3 years ago
Comment on attachment 8601422 [details] [diff] [review]
Bug1138288 v2

Review of attachment 8601422 [details] [diff] [review]:
-----------------------------------------------------------------

I get what you're doing with renaming mVirtualBounds -> mScreenBounds and mScreenBounds to mNaturalBounds. The real trouble I think comes just from mScreenBounds, though - it's not obvious whether it's referring to the virtual screen that the app uses, or the "real" screen that GLES and the rest of the graphics stack sees. So I would recommend only doing one renaming - mScreenBounds to mNaturalBounds. Or anything else is probably fine, as long as it's not as ambiguous as mScreenBounds.

::: widget/gonk/nsWindow.cpp
@@ +902,3 @@
>  // nsScreenGonk.cpp
>  
> +nsScreenGonk::nsScreenGonk(uint32_t aId, android::sp<ANativeWindow> aNativeWindow)

Having a smart pointer as an arg is a bit weird. I think we should go with a raw pointer here.
Attachment #8601422 - Flags: review?(mwu) → review+
(Assignee)

Comment 28

3 years ago
(In reply to Michael Wu [:mwu] from comment #27)
> Comment on attachment 8601422 [details] [diff] [review]
> Bug1138288 v2
> 
> Review of attachment 8601422 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I get what you're doing with renaming mVirtualBounds -> mScreenBounds and
> mScreenBounds to mNaturalBounds. The real trouble I think comes just from
> mScreenBounds, though - it's not obvious whether it's referring to the
> virtual screen that the app uses, or the "real" screen that GLES and the
> rest of the graphics stack sees. So I would recommend only doing one
> renaming - mScreenBounds to mNaturalBounds. Or anything else is probably
> fine, as long as it's not as ambiguous as mScreenBounds.
> 
> ::: widget/gonk/nsWindow.cpp
> @@ +902,3 @@
> >  // nsScreenGonk.cpp
> >  
> > +nsScreenGonk::nsScreenGonk(uint32_t aId, android::sp<ANativeWindow> aNativeWindow)
> 
> Having a smart pointer as an arg is a bit weird. I think we should go with a
> raw pointer here.

Thanks for the review!
(Assignee)

Comment 29

3 years ago
Created attachment 8602545 [details] [diff] [review]
Bug1138288 v3 (r+'d)
Attachment #8601422 - Attachment is obsolete: true
(Assignee)

Comment 30

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=43f2f4e45391
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 31

3 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/c3446a630074
Keywords: checkin-needed
(Reporter)

Comment 32

3 years ago
Why is this not merge to m-c yet? If something went wrong please let us know.
Flags: needinfo?(ryanvm)
Looks like this got merged last Thursday and the bug didn't get marked.
Flags: needinfo?(ryanvm)
https://hg.mozilla.org/mozilla-central/rev/c3446a630074
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S12 (15may)
Comment on attachment 8602545 [details] [diff] [review]
Bug1138288 v3 (r+'d)

Review of attachment 8602545 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/gonk/nsWindow.cpp
@@ +1280,5 @@
> +    mScreens.AppendElement(screen);
> +}
> +
> +void
> +nsScreenManagerGonk::RemoveScreen(uint32_t aDisplayType)

Doesn't the argument prevent support concurrent multiple VirtualDisplaySurfaces usage, does it?
You need to log in before you can comment on or make changes to this bug.