[Webkit-unassigned] [Bug 21322] DumpRenderTree pixel test improvements

[bugzilla-daemon at webkit.org]

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


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #24631|review?                     |review+
               Flag|                            |




------- Comment #38 from mitz at webkit.org  2008-10-26 23:11 PDT -------
(From update of attachment 24631)
> +        - Ensure font smoothing is disabled (this is also called LCD anti-aliasing and
> +        is different from regular font CG anti-aliasing) as font-smoothing settings
> +        depends on the display and can also be changed by the user

The user setting was overridden by the old code by setting the user default
value to MediumFontSmoothing, so I don't think that was part of the problem.

> +        - Use a new cleared buffer for each test instead of the reusing same one to
> +        avoid potential result corruption across tests

Is there any evidence that such corruption occurred or that it could occur? It
is OK to make this change, but it is not clear that it is solving a problem,
not even a potential one.

> +                }
> +            }
> +            else
> +                fputs("error, test and reference image have different properties.\n", stderr);

The else should go on the same line with the brace.

> +    static PassRefPtr<BitmapContext> createByAdoptingBitmapAndContext(BitmapContextBacking backing, CGContextRef context)

I think "BitmapContextBacking" and "backing" are not ideal names. Variable
names should almost always be nouns, and the noun meaning of "backing" (the
gerund) does not make sense here. "Backing buffer" or "backing store" seem
better for the variable names, while something "PlatformBitmapBuffer" might be
better as the type name.

These are minor comments so I am going to say r=me.


-- 
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.