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

[bugzilla-daemon at webkit.org]

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





------- Comment #39 from pol at apple.com  2008-10-27 11:50 PDT -------
(In reply to comment #38)
> (From update of attachment 24631 [edit])
> > +        - 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.

Actually, it's better not to have any font smoothing at all, as it's only
available in some circumstances and can change across OS releases. There were
also some bases that were checked in with no font smoothing, while the majority
has it.

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

No evidence, but if for whatever reason a test page doesn't full paint opaquely
the context, it will blend with the previous result.

> > +                }
> > +            }
> > +            else
> > +                fputs("error, test and reference image have different properties.\n", stderr);
> 
> The else should go on the same line with the brace.

Fixed

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

Fixed


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