Closed Bug 929471 Opened 11 years ago Closed 11 years ago

Make DrawTargetCG independent from QuartzSupport.mm

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Details

(Whiteboard: [qa-])

Attachments

(2 files, 1 obsolete file)

      No description provided.
Attached patch An attempt (obsolete) — Splinter Review
What do you think of this?
Attachment #820340 - Flags: review?(bgirard)
Don't forget license headers
(In reply to :Ms2ger from comment #2)
> Don't forget license headers

Are you running a patch reviewer bot locally? :D
Comment on attachment 820340 [details] [diff] [review]
An attempt

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

Would be nice if we killed IOSurface sym stuff with this patch.

::: gfx/2d/MacIOSurface.cpp
@@ +2,5 @@
> +#include <QuartzCore/QuartzCore.h>
> +#include <dlfcn.h>
> +#include "mozilla/RefPtr.h"
> +
> +using namespace mozilla;

I think it's time to kill most of this code (except maybe the IOSurfaceQuartz bits). We no longer support 10.5 which was the reason for all this.

@@ +47,5 @@
> +CFStringRef                   MacIOSurfaceLib::kPropHeight;
> +CFStringRef                   MacIOSurfaceLib::kPropBytesPerElem;
> +CFStringRef                   MacIOSurfaceLib::kPropBytesPerRow;
> +CFStringRef                   MacIOSurfaceLib::kPropIsGlobal;
> +#

Is this valid c++?

@@ +55,5 @@
> +  // if it is not available.
> +  if (!isLoaded)
> +    LoadLibrary();
> +  if (!sIOSurfaceFramework) {
> +    //NS_ERROR("MacIOSurfaceLib failed to initialize");

You have code changes in the move. Can you tell me what's changed? moz should support mfbt asserts.

@@ +255,5 @@
> +}
> +
> +TemporaryRef<MacIOSurface> MacIOSurface::LookupSurface(IOSurfaceID aIOSurfaceID,
> +                                                       double aContentsScaleFactor,
> +                                                       bool aHasAlpha) { 

Since you're touching these lines it would be a good time to fix these whitespaces.

::: gfx/2d/MacIOSurface.h
@@ +169,5 @@
> +      CloseLibrary();
> +    }
> +  } sLibraryUnloader;
> +};
> +#if 0

remove this.

::: gfx/gl/SharedSurfaceIO.cpp
@@ +75,5 @@
>      mGL->fTexParameteri(LOCAL_GL_TEXTURE_RECTANGLE_ARB,
>                          LOCAL_GL_TEXTURE_WRAP_T,
>                          LOCAL_GL_CLAMP_TO_EDGE);
>  
> +    CGLContextObj ctx = static_cast<CGLContextObj>(mGL->GetNativeData(GLContext::NativeCGLContext));

This change doesn't exactly belong here but that's ok.
Attachment #820340 - Flags: review?(bgirard) → review-
Can we reduce this in a follow up so we can land it soon?
Attachment #820340 - Attachment is obsolete: true
Attachment #822343 - Flags: review?(bgirard)
Attachment #822343 - Flags: review?(bgirard) → review+
https://hg.mozilla.org/mozilla-central/rev/ca7e5bb28550
https://hg.mozilla.org/mozilla-central/rev/82bc6c4f7e5b
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
This breaks the build for me on 10.9 due to GL_TEXTURE_RECTANGLE_ARB not being defined. Also, it looks like MacIOSurface.cpp landed without the license header.
Attachment #829626 - Flags: review?(bgirard)
I already fixed this on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/79b22871f81e

Sorry about the breakage.
Attachment #829626 - Flags: review?(bgirard) → review+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.