Convert GetSurfaceForGdkDrawable to return a DrawTarget

RESOLVED FIXED in Firefox 50



3 years ago
2 years ago


(Reporter: jwatt, Assigned: jwatt)


Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)



(2 attachments, 1 obsolete attachment)



3 years ago
As part of getting rid of gfxXlibSurface (and ultimately gfxASurface) I have a patch that pushes the DrawTarget wrapping of the gfxXlibSurface created by GetSurfaceForGdkDrawable back to just after it is created (rather than passing it out to callers and having it wrapped later).

Once we get to a point where all gfxXlibSurface surfaces are immediately wrapped it becomes easy to replace them all with a DrawTarget that directly wraps a cairo_xlib_surface_t.

Comment 1

3 years ago
Created attachment 8760256 [details] [diff] [review]
Attachment #8760256 - Flags: review?(bas)
Comment on attachment 8760256 [details] [diff] [review]

Review of attachment 8760256 [details] [diff] [review]:

::: widget/gtk/nsWindow.cpp
@@ +6462,5 @@
>      return keyBindings->Execute(aEvent, aCallback, aCallbackData);
>  }
>  #if defined(MOZ_X11) && (MOZ_WIDGET_GTK == 2)
>  /* static */ already_AddRefed<gfxASurface>

I don't see how this compiles since the return value here is wrong.
Attachment #8760256 - Flags: review?(bas) → review-

Comment 3

3 years ago
Good catch. I guess that must mean all our linux builders on Try are using gtk3 now.

Comment 4

3 years ago
Created attachment 8760347 [details] [diff] [review]
Attachment #8760256 - Attachment is obsolete: true
Attachment #8760347 - Flags: review?(bas)

Comment 5

3 years ago
FWIW I need this patch to make progress on tier 1 work, but karlt considers this code tier 2 - we don't bother to build and test it ourselves, but we should try to write code that will work for it:
Attachment #8760347 - Flags: review?(bas) → review+

Comment 6

3 years ago
Pushed by
Convert GetSurfaceForGdkDrawable to return a DrawTarget. r=Bas

Comment 7

3 years ago
Last Resolved: 3 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50

Comment 8

3 years ago
/media/data/develop/mozilla/central/widget/gtk/nsDragService.cpp:446:48: error: no matching constructor for initialization of 'IntRect'

Comment 9

3 years ago
Created attachment 8762689 [details] [diff] [review]
potential compilation fix

Comment 10

3 years ago
Does the patch is comment 9 fix things?
Flags: needinfo?(c)

Comment 11

3 years ago
I can compile it with the patch.
Flags: needinfo?(c)
Depends on: 1281074
(In reply to Jonathan Watt [:jwatt] from comment #9)
> Created attachment 8762689 [details] [diff] [review]
> potential compilation fix

This patch does not fix the compilation issue for me.  I additionally need this as well:

diff --git a/widget/gtk/nsWindow.h b/widget/gtk/nsWindow.h
--- a/widget/gtk/nsWindow.h
+++ b/widget/gtk/nsWindow.h
@@ -316,17 +316,17 @@ public:
    nsresult            UpdateTranslucentWindowAlphaInternal(const nsIntRect& aRect,
                                                             uint8_t* aAlphas, int32_t aStride);
     already_AddRefed<mozilla::gfx::DrawTarget> GetDrawTarget(const LayoutDeviceIntRegion& aRegion,
                                                              mozilla::layers::BufferMode* aBufferMode);
 #if (MOZ_WIDGET_GTK == 2)
     static already_AddRefed<DrawTarget> GetDrawTargetForGdkDrawable(GdkDrawable* aDrawable,
-                                                                    const IntSize& aSize);
+                                                                    const nsIntSize& aSize);
     NS_IMETHOD         ReparentNativeWidget(nsIWidget* aNewParent) override;
     virtual nsresult SynthesizeNativeMouseEvent(LayoutDeviceIntPoint aPoint,
                                                 uint32_t aNativeMessage,
                                                 uint32_t aModifierFlags,
                                                 nsIObserver* aObserver) override;
Depends on: 1280964
You need to log in before you can comment on or make changes to this bug.