The default bug view has changed. See this FAQ.

Make NPAPI Async plugin drawing scale correctly

RESOLVED FIXED in mozilla13

Status

()

Core
Plug-ins
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

Trunk
mozilla13
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 602921 [details] [diff] [review]
Part 1: Add ScaleToSize API to ImageLayer

The NPAPI async spec says plugin surfaces should be scaled to cover the plugin child area.
Attachment #602921 - Flags: review?(roc)
(Assignee)

Comment 1

5 years ago
Created attachment 602922 [details] [diff] [review]
Part 2: Scale plugin surfaces to client area size
Attachment #602922 - Flags: review?(roc)
Comment on attachment 602921 [details] [diff] [review]
Part 1: Add ScaleToSize API to ImageLayer

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

::: gfx/layers/ImageLayers.h
@@ +583,5 @@
>  public:
> +  enum ScaleMode {
> +    SCALE_NONE,
> +    SCALE_STRETCH,
> +    SCALE_PRESERVE_ASPECT_RATIO_CONTAIN

Let's not add this (or comment it out) until it's supported.

And add a comment that STRETCH is not handled on GL or e10s yet.

@@ +607,5 @@
> +   * Set the size to scale the image to and the mode at which to scale.
> +   */
> +  void SetScaleToSize(const gfxIntSize &aSize, ScaleMode aMode)
> +  {
> +    mSize = aSize; mScaleMode = aMode;

two lines

@@ +645,3 @@
>    nsRefPtr<ImageContainer> mContainer;
>    gfxPattern::GraphicsFilter mFilter;
> +  gfxIntSize mSize;

Better call this "mScaleToSize" to avoid confusion. It's not the size of the image (UNLIKE mSize in BasicImageLayer).

::: gfx/layers/basic/BasicLayers.cpp
@@ -848,5 @@
>  class BasicImageLayer : public ImageLayer, public BasicImplData {
>  public:
>    BasicImageLayer(BasicLayerManager* aLayerManager) :
> -    ImageLayer(aLayerManager, static_cast<BasicImplData*>(this)),
> -    mSize(-1, -1)

You're removing this, but BasicShadowableImageLayer::Paint is still using it and ends up using the mSize in ImageLayer which is wrong!

@@ +929,5 @@
>    // The visible region can extend outside the image.  If we're not
>    // tiling, we don't want to draw into that area, so just draw within
>    // the image bounds.
>    const nsIntRect* tileSrcRect = GetTileSourceRect();
> +  AutoSetOperator setOpera7tor(aContext, GetOperator());

fix typo!

::: gfx/layers/d3d10/ImageLayerD3D10.cpp
@@ +129,5 @@
>    if (!image) {
>      return;
>    }
>  
> +  gfxIntSize size = mScaleMode == SCALE_NONE ?  image->GetSize() : mSize;

only one space after ?

::: gfx/layers/d3d9/ImageLayerD3D9.cpp
@@ +324,5 @@
>    }
>  
>    SetShaderTransformAndOpacity();
>  
> +  gfxIntSize size = mScaleMode == SCALE_NONE ?  image->GetSize() : mSize;

only one space after ?
Attachment #602921 - Flags: review?(roc) → review-
Attachment #602922 - Flags: review?(roc) → review+
(Assignee)

Comment 3

5 years ago
Created attachment 604424 [details] [diff] [review]
Part 1: Add ScaleToSize API to ImageLayer v2
Attachment #602921 - Attachment is obsolete: true
Attachment #604424 - Flags: review?(roc)
Comment on attachment 604424 [details] [diff] [review]
Part 1: Add ScaleToSize API to ImageLayer v2

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

::: gfx/layers/ImageLayers.h
@@ +646,3 @@
>    nsRefPtr<ImageContainer> mContainer;
>    gfxPattern::GraphicsFilter mFilter;
> +  gfxIntSize mScaleToSize;

Comment that this is only valid of mScaleMode != SCALE_NONE.
Attachment #604424 - Flags: review?(roc) → review+
We need ShadowLayers support for this... who's going to do it and when?
(Assignee)

Comment 6

5 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) (away February 21 to March 17) from comment #5)
> We need ShadowLayers support for this... who's going to do it and when?

It's a good question, I guess right now we don't use shadowlayers on any platform which supports it. But we will need it in the future. Benoit Girard might be a good person as he works on OMTC and might also work on the accelerated drawing model for Mac so will need to get in touch with some of the new plugin drawing code.
I guess one big question is who's going to do OMTC for Windows.
(Assignee)

Comment 8

5 years ago
Created attachment 604776 [details] [diff] [review]
Part 2: Scale plugin surfaces to client area size

Updated to work with Mac drawing models as per e-mail discussion.
Attachment #602922 - Attachment is obsolete: true
Attachment #604776 - Flags: review?(roc)
Attachment #604776 - Flags: review?(roc) → review+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b46f6eff8f4
https://hg.mozilla.org/integration/mozilla-inbound/rev/f667f6c22bed
Backed out:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/7bbff0b46460
  https://hg.mozilla.org/integration/mozilla-inbound/rev/52e11f6c80b9
for having apparently caused a Linux opt Cipc crashtest failure.

Sample logs:
https://tbpl.mozilla.org/php/getParsedLog.php?id=9990114&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=9990076&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=9989912&tree=Mozilla-Inbound

The failing test is:
  http://mxr.mozilla.org/mozilla-central/source/layout/base/crashtests/50395-1.html?force=1

The crash is us hitting the final NS_RUNTIMEABORT() in BasicShadowableImageLayer::Paint and aborting.
Version: unspecified → Trunk
csets for landing & backout merged to m-c.

Landing:
 https://hg.mozilla.org/mozilla-central/rev/4b46f6eff8f4
 https://hg.mozilla.org/mozilla-central/rev/f667f6c22bed
Backout:
 https://hg.mozilla.org/mozilla-central/rev/7bbff0b46460
 https://hg.mozilla.org/mozilla-central/rev/52e11f6c80b9
(Assignee)

Comment 12

5 years ago
Created attachment 605043 [details] [diff] [review]
Part 1: Add ScaleToSize API to ImageLayer v3

The old patch had an issue with shadowableImageLayer where mSize would not be updated correctly. This patch fixes that.
Attachment #604424 - Attachment is obsolete: true
Attachment #605043 - Flags: review?(roc)
Attachment #605043 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/709bce4da141
https://hg.mozilla.org/mozilla-central/rev/babf5e9f6036
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.