[Webkit-unassigned] [Bug 73040] FloatRect::isRectilinear() returns false for 180degree rotations

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 23 14:12:09 PST 2011


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





--- Comment #5 from David Reveman <reveman at chromium.org>  2011-11-23 14:12:09 PST ---
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 116392 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=116392&action=review
> > 
> > Good catch! This will also increase the number of cases where we can avoid the anti-aliasing shader.
> > 
> > > Source/WebCore/platform/graphics/FloatQuad.cpp:90
> > > +    return a - b < 0.00000000000001 && a - b > -0.00000000000001;
> > 
> > I'd put that magic value in a constant:
> > const float epsilon = 0.00000000000001;
> 
> Sure.
> 
> > that's enough to make sure there's no confusion about its use. Maybe it also doesn't have to be that small? It just has to be small enough to not affect rendering, right? We have to make a similar check in CCTiledLayerImpl::drawTiles. I used 1 / 1024 there.
> 
> I guess this would be the right value to use in both cases: http://msdn.microsoft.com/en-us/library/6x7575x3(v=vs.71).aspx
> 
> > and maybe "return fabs(a - b) < epsilon" would be cleaner?
> 
> I think this has an extra floating point instead of boolean instruction (the fabs() vs &&), but I see it used numerous times in the code already so yup. Thanks.

Thanks, it's just for readability.

> 
> > > Source/WebCore/platform/graphics/FloatQuad.cpp:93
> > >  bool FloatQuad::isRectilinear() const
> > 
> > Maybe it's a bad idea to do this approximation in isRectilinear(). How about adding a FloatQuad::isCloseToRectilinear()?
> 
> Are there places that would want to check isRectilinear and expect a 180 degree rotation to fail ?

No need for a isCloseToRectilinear if you use std::numeric_limits<float>::epsilon() like in the current patch. That would only make sense if we rounded based on pixel precision. 

Current patch looks good to me.

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



More information about the webkit-unassigned mailing list