1220629 - Implement PushLayer/PopLayer APIs in Moz2D

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

We want to implement Push and PopLayer as first class citizens in Moz2D. This will allow for better performance by allowing DrawTargets to use internal optimizations for these calls rather than forcing the creation of temporary DrawTargets.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Add PushLayer/PopLayer API to DrawTarget baseclass.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 1: Add PushLayer/PopLayer API to DrawTarget baseclass. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29427/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29427/

Comment 3

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 2: Prepare DrawTargetD2D1 for the possibilities of layers existing inside it. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29429/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29429/

Comment 4

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 3: Implement PushLayer/PopLayer API in cairo. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29431/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29431/

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 4: Allow gfxContext to use the native pushlayer implementations based on a pref. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29433/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29433/

Comment 6

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 5: Implement PushLayer/PopLayer API for Direct2D 1.1. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29435/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29435/

Comment 7

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 6: Implement PushLayer/PopLayer API in several wrapper DT types. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29437/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29437/

Comment 8

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 7: Mark several reftests fuzzy. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29439/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29439/

Comment 9

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1220629 - Part 8: Enable native PushLayer/PopLayer by default on Windows and Linux. r=jrmuizel

Review commit: https://reviewboard.mozilla.org/r/29441/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29441/

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703665 [details]
MozReview Request: Bug 1220629 - Part 2: Prepare DrawTargetD2D1 for the possibilities of layers existing inside it. r=jrmuizel

https://reviewboard.mozilla.org/r/29429/#review26227

Comment 11

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703666 [details]
MozReview Request: Bug 1220629 - Part 3: Implement PushLayer/PopLayer API in cairo. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29431/diff/1-2/

Comment 12

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703667 [details]
MozReview Request: Bug 1220629 - Part 4: Allow gfxContext to use the native pushlayer implementations based on a pref. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29433/diff/1-2/

Comment 13

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703668 [details]
MozReview Request: Bug 1220629 - Part 5: Implement PushLayer/PopLayer API for Direct2D 1.1. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29435/diff/1-2/

Comment 14

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703669 [details]
MozReview Request: Bug 1220629 - Part 6: Implement PushLayer/PopLayer API in several wrapper DT types. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29437/diff/1-2/

Comment 15

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703670 [details]
MozReview Request: Bug 1220629 - Part 7: Mark several reftests fuzzy. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29439/diff/1-2/

Comment 16

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703678 [details]
MozReview Request: Bug 1220629 - Part 8: Enable native PushLayer/PopLayer by default on Windows and Linux. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29441/diff/1-2/

Comment 17

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703666 [details]
MozReview Request: Bug 1220629 - Part 3: Implement PushLayer/PopLayer API in cairo. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29431/diff/2-3/

Comment 18

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703667 [details]
MozReview Request: Bug 1220629 - Part 4: Allow gfxContext to use the native pushlayer implementations based on a pref. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29433/diff/2-3/

Comment 19

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703668 [details]
MozReview Request: Bug 1220629 - Part 5: Implement PushLayer/PopLayer API for Direct2D 1.1. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29435/diff/2-3/

Comment 20

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703669 [details]
MozReview Request: Bug 1220629 - Part 6: Implement PushLayer/PopLayer API in several wrapper DT types. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29437/diff/2-3/

Comment 21

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703670 [details]
MozReview Request: Bug 1220629 - Part 7: Mark several reftests fuzzy. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29439/diff/2-3/

Comment 22

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703678 [details]
MozReview Request: Bug 1220629 - Part 8: Enable native PushLayer/PopLayer by default on Windows and Linux. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29441/diff/2-3/

Comment 23

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703664 [details]
MozReview Request: Bug 1220629 - Part 1: Add PushLayer/PopLayer API to DrawTarget baseclass. r=jrmuizel

https://reviewboard.mozilla.org/r/29427/#review26387

Comment 24

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703666 [details]
MozReview Request: Bug 1220629 - Part 3: Implement PushLayer/PopLayer API in cairo. r=jrmuizel

https://reviewboard.mozilla.org/r/29431/#review26173

::: gfx/2d/DrawTargetCairo.cpp:1543
(Diff revision 1)
> +  }

Lifting this if condition inside the one above makes it easier for me to understand what's going on.


  } else {
    if (layer.mOpacity != Float(1.0)) {
        cairo_push_group_with_content(mContext, CAIRO_CONTENT_COLOR_ALPHA);

        // Now draw the content using the desired operator
        cairo_paint_with_alpha(mContext, layer.mOpacity);

        cairo_pop_group_to_source(mContext);
    }
    cairo_mask(mContext, layer.mMaskPattern);
  }

Comment 25

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703667 [details]
MozReview Request: Bug 1220629 - Part 4: Allow gfxContext to use the native pushlayer implementations based on a pref. r=jrmuizel

https://reviewboard.mozilla.org/r/29433/#review26399

Comment 26

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703669 [details]
MozReview Request: Bug 1220629 - Part 6: Implement PushLayer/PopLayer API in several wrapper DT types. r=jrmuizel

https://reviewboard.mozilla.org/r/29437/#review26401

Comment 27

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703670 [details]
MozReview Request: Bug 1220629 - Part 7: Mark several reftests fuzzy. r=jrmuizel

https://reviewboard.mozilla.org/r/29439/#review26403

Comment 28

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703678 [details]
MozReview Request: Bug 1220629 - Part 8: Enable native PushLayer/PopLayer by default on Windows and Linux. r=jrmuizel

https://reviewboard.mozilla.org/r/29441/#review26405

Comment 29

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8703668 [details]
MozReview Request: Bug 1220629 - Part 5: Implement PushLayer/PopLayer API for Direct2D 1.1. r=jrmuizel

https://reviewboard.mozilla.org/r/29435/#review26407

::: gfx/2d/DrawTargetD2D.cpp:2724
(Diff revision 3)
> -  //options.debugLevel = D2D1_DEBUG_LEVEL_INFORMATION;
> +  options.debugLevel = D2D1_DEBUG_LEVEL_INFORMATION;

Was this change intentional?

::: gfx/2d/DrawTargetD2D1.cpp:762
(Diff revision 3)
> +      image->QueryInterface((ID2D1Bitmap**)getter_AddRefs(bitmap));

What if image is not a ID2D1Bitmap? Do we ever have non ID2D1Bitmaps in SourceSurfaces?

::: gfx/2d/DrawTargetD2D1.cpp:769
(Diff revision 3)
> +      clip->Open(getter_AddRefs(sink));

It'd be nice to have a helper that takes a rect and adds it to the path, instead of open coding this inline.

::: gfx/2d/HelpersD2D.h:840
(Diff revision 3)
> +    

There's a bunch of these trailing spaces.

Comment 30

[Bas Schouten (:bas.schouten)]

https://reviewboard.mozilla.org/r/29435/#review26407

> Was this change intentional?

Nope, you're absolutely right!

> What if image is not a ID2D1Bitmap? Do we ever have non ID2D1Bitmaps in SourceSurfaces?

No, we already have places which depend on this.

Comment 31

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703664 [details]
MozReview Request: Bug 1220629 - Part 1: Add PushLayer/PopLayer API to DrawTarget baseclass. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29427/diff/1-2/

Comment 32

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703665 [details]
MozReview Request: Bug 1220629 - Part 2: Prepare DrawTargetD2D1 for the possibilities of layers existing inside it. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29429/diff/1-2/

Comment 33

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703666 [details]
MozReview Request: Bug 1220629 - Part 3: Implement PushLayer/PopLayer API in cairo. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29431/diff/3-4/

Comment 34

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703667 [details]
MozReview Request: Bug 1220629 - Part 4: Allow gfxContext to use the native pushlayer implementations based on a pref. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29433/diff/3-4/

Comment 35

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703668 [details]
MozReview Request: Bug 1220629 - Part 5: Implement PushLayer/PopLayer API for Direct2D 1.1. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29435/diff/3-4/

Comment 36

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703669 [details]
MozReview Request: Bug 1220629 - Part 6: Implement PushLayer/PopLayer API in several wrapper DT types. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29437/diff/3-4/

Comment 37

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703670 [details]
MozReview Request: Bug 1220629 - Part 7: Mark several reftests fuzzy. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29439/diff/3-4/

Comment 38

[Bas Schouten (:bas.schouten)]

Comment on attachment 8703678 [details]
MozReview Request: Bug 1220629 - Part 8: Enable native PushLayer/PopLayer by default on Windows and Linux. r=jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29441/diff/3-4/

Comment 39

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8bd590cba7fd
https://hg.mozilla.org/integration/mozilla-inbound/rev/97be7e6c0313
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d22446ef1ba
https://hg.mozilla.org/integration/mozilla-inbound/rev/853ea2b1f19d
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd2e3fed3792
https://hg.mozilla.org/integration/mozilla-inbound/rev/4358152749cf
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ad8cc29306f
https://hg.mozilla.org/integration/mozilla-inbound/rev/d74084d072f0

Comment 40

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8bd590cba7fd
https://hg.mozilla.org/mozilla-central/rev/97be7e6c0313
https://hg.mozilla.org/mozilla-central/rev/7d22446ef1ba
https://hg.mozilla.org/mozilla-central/rev/853ea2b1f19d
https://hg.mozilla.org/mozilla-central/rev/fd2e3fed3792
https://hg.mozilla.org/mozilla-central/rev/4358152749cf
https://hg.mozilla.org/mozilla-central/rev/5ad8cc29306f
https://hg.mozilla.org/mozilla-central/rev/d74084d072f0