[Webkit-unassigned] [Bug 16979] Patch to conditionalize some CG/Cairo support in win32

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=16979


aroben at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18768|review?                     |review-
               Flag|                            |




------- Comment #27 from aroben at apple.com  2008-01-29 12:47 PDT -------
(From update of attachment 18768)
- * Copyright (C) 2006, 2007 Apple Inc.  All rights reserved.
+ * Copyright (C) 2008 Apple Inc.  All rights reserved.

I think you need to retain the "2006, 2007" for the new *CGWin.cpp files, since
the code in them (which is what the copyright pertains to) dates back to 2006.

+HBITMAP imageFromSelection(Frame* frame, bool forceBlackText)
+{
+    notImplemented();
+}

You're missing a return statement here.

+    setPaintingDisabled(!m_data->m_cgContext);
+    if (m_data->m_cgContext) {

Seems strange to have a member called m_cgContext in
GraphicsContextCairoWin.cpp.

+    size_t imageHeight = 0;
+    size_t imageWidth = 0;
     for (int i = 0; i < frames; ++i) {
-        CGImageRef image = frameAtIndex(i);
-        if (CGImageGetHeight(image) == (size_t)srcSize.height() &&
CGImageGetWidth(image) == (size_t)srcSize.width()) {
+        NativeImagePtr image = frameAtIndex(i);
+        imageHeight = CGImageGetHeight(image);
+        imageWidth = CGImageGetWidth(image);
+        if (imageHeight == static_cast<size_t>(srcSize.height()) && imageWidth
== static_cast<size_t>(srcSize.width())) {

I don't think there's any reason to move the imageHeight and imageWidth
declarations out of the for loop. We also don't tend to use typedefs like
NativeImagePtr within the source files, just in the headers, so you should
change that back to CGImageRef. Ditto for ImageCairoWin.cpp.

+    SelectObject(tempDC, bmp);
+    GraphicsContext gc(tempDC);
+#endif

I don't see what #if or #ifdef this #endif is matching.

Are the *CairoWin.cpp files supposed to compile yet? Or is this just an initial
non-compiling-but-close version? I think landing a compiling version is vastly
preferable to a non-compiling one.

I'm going to r- so that we can get these last few niggling details worked out,
but then this should be good to go!


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.