Bug 87915 - [chromium] Fix min/max bounds error in CCMathUtil.cpp
Summary: [chromium] Fix min/max bounds error in CCMathUtil.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Shawn Singh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-30 18:24 PDT by Shawn Singh
Modified: 2012-05-30 23:05 PDT (History)
4 users (show)

See Also:


Attachments
Patch (15.06 KB, patch)
2012-05-30 18:31 PDT, Shawn Singh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shawn Singh 2012-05-30 18:24:27 PDT
To compute bounds of vertices in CCMathUtil.cpp, there were two places where min/max values were assigned to initial opposite max/min values.

There was a mistake in this code however, because numeric_limits<float>::min() is actually zero, not -flt_max.  The desired correct behavior is to use -flt_max.

Patch coming in a moment:
  - moves some code around so that unit testing is nice and clean
  - fixes the numeric_limits mistake in two places (computeEnclosingRectForVertices and computeEnclosingClippedRect)
  - adds two unit tests to cover the change.
Comment 1 Shawn Singh 2012-05-30 18:31:42 PDT
Created attachment 144967 [details]
Patch
Comment 2 James Robinson 2012-05-30 18:43:04 PDT
Comment on attachment 144967 [details]
Patch

numeric_limits<float>::min() is actually something like the least positive representable number (can't remember if it's a denormal or not).  Pretty much never what you want.

Looks good, do we expect any impact on pages?
Comment 3 Shawn Singh 2012-05-30 19:13:41 PDT
Comment on attachment 144967 [details]
Patch

(In reply to comment #2)
> (From update of attachment 144967 [details])
> numeric_limits<float>::min() is actually something like the least positive representable number (can't remember if it's a denormal or not).  Pretty much never what you want.
> 
> Looks good, do we expect any impact on pages?

Thanks for the review

I wouldn't be surprised if a few rare uncommon pages broke because of this, but I have not actually encountered them.  On pages that have been already working, there would not be any performance benefit or any other change.
Comment 4 WebKit Review Bot 2012-05-30 23:05:47 PDT
Comment on attachment 144967 [details]
Patch

Clearing flags on attachment: 144967

Committed r119058: <http://trac.webkit.org/changeset/119058>
Comment 5 WebKit Review Bot 2012-05-30 23:05:51 PDT
All reviewed patches have been landed.  Closing bug.