Closed Bug 916034 Opened 6 years ago Closed Last year

Enable Azure content for gtk2

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED INVALID

People

(Reporter: mattwoodrow, Unassigned)

References

Details

(Whiteboard: [leave open])

Attachments

(4 files, 3 obsolete files)

No description provided.
We had the pattern transform as the inverse of what we needed previously.

Just switching to MaskSurface matches the Thebes path better, and fixes the bug.
Attachment #804325 - Flags: review?(jmuizelaar)
Attached patch Fix GetCairoSplinter Review
Native drawing that tries to use this really wants the current group surface, not the original.
Attachment #804327 - Flags: review?(jmuizelaar)
Attached patch Clip native drawing (obsolete) — Splinter Review
Cairo's default extend mode for drawing images is EXTEND_NONE, but azure doesn't support that. So the Thebes wrapper just uses EXTEND_PAD instead...

This worries me a little bit, since we have hundreds of callers to SetSource(gfxASurface*) that will be expecting NONE.

Anyway, this fixes this one by clipping the drawing area.
Attachment #804329 - Flags: review?(jmuizelaar)
Attached patch Create a DrawTarget in nsWindow (obsolete) — Splinter Review
Attachment #804331 - Flags: review?(jmuizelaar)
Looks like this regresses TART too. I guess I'll try fix that before landing this.
Comment on attachment 804325 [details] [diff] [review]
Use MaskSurface in DrawBufferQuadrant

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

::: gfx/layers/ThebesLayerBuffer.cpp
@@ +205,5 @@
>    if (aMask) {
> +    Matrix oldTransform = aTarget->GetTransform();
> +    aTarget->SetTransform(*aMaskTransform);
> +    aTarget->MaskSurface(source, aMask, Point(0, 0), DrawOptions(aOpacity, aOperator));
> +    aTarget->SetTransform(oldTransform);

Does the source pattern transform need to adjusted?
Attachment #804325 - Flags: review?(jmuizelaar) → review+
Attachment #804331 - Flags: review?(jmuizelaar) → review+
Comment on attachment 804333 [details] [diff] [review]
Enable

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

The code change here is fine. Does this regress anything?
Attachment #804333 - Flags: review?(jmuizelaar) → review+
Comment on attachment 804329 [details] [diff] [review]
Clip native drawing

No patch.
Attachment #804329 - Flags: review?(jmuizelaar) → review-
Comment on attachment 804327 [details] [diff] [review]
Fix GetCairo

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

::: gfx/thebes/gfxContext.cpp
@@ +195,5 @@
>    }
>  
>    if (mDT->GetType() == BACKEND_CAIRO) {
>      cairo_t *ctx =
> +      (cairo_t*)mDT->GetNativeSurface(NATIVE_SURFACE_CAIRO_CONTEXT);

Assuming this is the only intended hunk. This seems sane.

::: gfx/thebes/gfxXlibNativeRenderer.cpp
@@ +490,1 @@
>  

Is this hunk intended for another patch? Either way it doesn't seem to change any behaviour...
Attachment #804327 - Flags: review?(jmuizelaar) → review+
> 
> Is this hunk intended for another patch? Either way it doesn't seem to
> change any behaviour...

It does! It changes the scope of the gfxContextAutoSaveRestore.

Previously we only set a clip when drawing directly to the destination surface, and removed it regardless if this succeed or not.

The fallback path (allocating a temporary, drawing to that and then blitting it back) was done without clipping.

This worked fine when the blit back was done with EXTEND_NONE, but Azure doesn't support that, and blitting back with EXTEND_PAD is obviously incorrect.

The patch just leaves the clip applied for both paths so we get correct behaviour.
Attached patch Clip native drawing v2 (obsolete) — Splinter Review
Attachment #804329 - Attachment is obsolete: true
Attachment #805079 - Flags: review?(jmuizelaar)
Attachment #805079 - Flags: review?(jmuizelaar) → review+
Attachment #804331 - Attachment is obsolete: true
Attachment #805079 - Attachment is obsolete: true
Attachment #804327 - Flags: checkin+
Attachment #804325 - Flags: checkin+
This was really slow on X11, since we had to upload to the GPU each time.
Attachment #809580 - Flags: review?(ajones)
Comment on attachment 809580 [details] [diff] [review]
Don't use an image surface for the cached canvas background image

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

::: layout/generic/nsCanvasFrame.cpp
@@ +246,5 @@
>          if (source) {
>            // Could be non-integer pixel alignment
> +          destDT->DrawSurface(source,
> +                              Rect(destRect.x, destRect.y, destRect.width, destRect.height),
> +                              Rect(0, 0, destRect.width, destRect.height));

This should be a call to BlitSurface() above.
Attachment #809580 - Flags: review?(ajones) → review+
Depends on: 923542
Bug 740598 should fix a lot of the tscroll regression.
Depends on: 740598
(In reply to Matt Woodrow (:mattwoodrow) from comment #20)
> Bug 740598 should fix a lot of the tscroll regression.

Bug 740598 fixed a portion of the tscroll regression, but it is still regressed by about 10% to 15%.  CanvasMark, TART, SVG, and SVG Opacity are also regressed.  Are there bugs tracking these regressions yet?
Bug 928123 fixed part of the remaining tscroll regression, and completely fixed the tresize regression.  (In fact tresize is now better than it was pre-azure.)

tscroll is still somewhat regressed, along with CanvasMark, TART, SVG, SVG Opacity.
Depends on: 928123
No, no bugs yet, because I have no leads :( Profiling hasn't shown anything interesting.

Looks like bug 928727 should help with tscrollx though.
Depends on: 934674
Blocks: 945088
Depends on: 971720
Depends on: 1015166
Depends on: 1048047
Closing GTK2 related bugs since we removed GTK2 support at the beginning of 2018 in bug 1278282. Probably best to open a new bug in the unlikely event that any of these are still relevant.
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.