[Webkit-unassigned] [Bug 26457] Build DumpRenderTree under Cairo

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26457


aroben at apple.com changed:

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




------- Comment #8 from aroben at apple.com  2009-07-02 09:47 PDT -------
(From update of attachment 32087)
> +2009-06-30  U-bfulgham-PC\bfulgham  <bfulgham at webkit.org>

Looks like you need to set REAL_NAME on this computer.

> +        * DumpRenderTree/PixelDumpSupport.cpp: Added.
> +        (dumpWebViewAsPixelsAndCompareWithExpected):
> +        (printPNG):
> +        * DumpRenderTree/PixelDumpSupport.h:
> +        * DumpRenderTree/cairo: Added.
> +        * DumpRenderTree/cairo/PixelDumpSupportCairo.cpp: Added.
> +        (writeFunction):
> +        (printPNG):
> +        (computeMD5HashStringForBitmapContext):
> +        (dumpBitmap):
> +        * DumpRenderTree/cairo/PixelDumpSupportCairo.h: Added.
> +        (BitmapContext::createByAdoptingBitmapAndContext):
> +        (BitmapContext::~BitmapContext):
> +        (BitmapContext::cairoContext):
> +        (BitmapContext::BitmapContext):
> +        * DumpRenderTree/cg/PixelDumpSupportCG.cpp:
> +        (printPNG):
> +        (computeMD5HashStringForBitmapContext):
> +        (dumpBitmap):
> +        * DumpRenderTree/cg/PixelDumpSupportCG.h:
> +        * DumpRenderTree/win/DumpRenderTree.cpp:
> +        (main):
> +        * DumpRenderTree/win/DumpRenderTree.vcproj:
> +        * DumpRenderTree/win/PixelDumpSupportWin.cpp:
> +        (createBitmapContextFromWebView):

It would be good to add more detailed comments about what you changed next to
each function/file.

> +#include "config.h"
> +#include "PixelDumpSupport.h"
> +#if PLATFORM(CG)
> +#include <CoreGraphics/CGBitmapContext.h>
> +#include "PixelDumpSupportCG.h"
> +#else
> +#include <cairo-win32.h>
> +#include "PixelDumpSupportCairo.h"
> +#endif
> +
> +#include "DumpRenderTree.h"
> +#include "LayoutTestController.h"
> +#include <wtf/Assertions.h>
> +#include <wtf/RefPtr.h>
> +#include <wtf/RetainPtr.h>

I'm not sure I understand why we need to include any port-specific headers
here. Can the declarations of the required functions move to
PixelDumpSupport.h?

> +#if PLATFORM(WIN)
> +static const CFStringRef kUTTypePNG = CFSTR("public.png");
> +#endif

I don't think this is needed in PixelDumpSupportCairo.

> +    const size_t dataLength = pixeldata.size ();
> +    const unsigned char* data = &pixeldata[0];
> +
> +    printPNG (dataLength, data);

Please remove the spaces after the function names.

pixeldata.data() would be better than &pixeldata[0]. pixeldata should probably
be renamed to pixelData.

> +void computeMD5HashStringForBitmapContext(RefPtr<BitmapContext> context, char hashString[33])

context's type should be BitmapContext*.

> +    // We need to swap the bytes to ensure consistent hashes independently of endianness

Your code doesn't seem to take endianness into account. Should this turn into a
FIXME? Be removed entirely?

> +void dumpBitmap(RefPtr<BitmapContext> context)

context's type should be BitmapContext*.

> +{
> +    cairo_surface_t* surface = cairo_get_target(context->cairoContext());
> +    printPNG(surface);
> +}
> \ No newline at end of file

Please add a newline.

> +    RetainPtr<CairoContextRef> m_context;

It seems unlikely that RetainPtr is the right smart pointer for holding a
CairoContextRef. Is it really right to use CFRetain/CFRelease with a
CairoContextRef?

> +#endif // PixelDumpSupportCairo_h
> \ No newline at end of file

Please add a newline.


-- 
Configure bugmail: https://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.