Bug 52910 - Add various clampToInt() methods to MathExtras.h
Summary: Add various clampToInt() methods to MathExtras.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-21 11:55 PST by Simon Fraser (smfr)
Modified: 2011-01-28 21:02 PST (History)
5 users (show)

See Also:

Attachments
Patch (4.23 KB, patch)
2011-01-21 12:23 PST, Simon Fraser (smfr) 		barraclough: review+
Details | Formatted Diff | Diff
Need this change too. (673 bytes, text/plain)
2011-01-28 17:49 PST, Simon Fraser (smfr) 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2011-01-21 11:55:54 PST 
In CSS code we need to range-check doubles before double-to-int conversion in various places. We need versions that allow only positive numbers, and positive and negative.
Comment 1 Simon Fraser (smfr) 2011-01-21 12:23:22 PST 
Created attachment 79774 [details]
Patch
Comment 2 Gavin Barraclough 2011-01-21 13:45:33 PST 
Comment on attachment 79774 [details]
Patch

r+,
but have you thought about how these methods will handle NaN? - I'm guessing that the answer is, we don't care for CSS - in which case I'd suggest ASSERT(!isNaN(d)) in each.  If you're not going to guard against NaN, I think you should document what expected behaviour is.
Comment 3 Build Bot 2011-01-21 14:12:55 PST 
Attachment 79774 [details] did not build on win:
Build output: http://queues.webkit.org/results/7515282
Comment 4 Simon Fraser (smfr) 2011-01-21 14:24:53 PST 
Gah, windows:

14>C:\cygwin\home\buildbot\Webkit\WebKitBuild\Debug\include\private\JavaScriptCore/MathExtras.h(213) : error C2220: warning treated as error - no 'object' file generated
14>C:\cygwin\home\buildbot\Webkit\WebKitBuild\Debug\include\private\JavaScriptCore/MathExtras.h(213) : warning C4003: not enough actual parameters for macro 'max'
14>C:\cygwin\home\buildbot\Webkit\WebKitBuild\Debug\include\private\JavaScriptCore/MathExtras.h(213) : error C2589: '(' : illegal token on right side of '::'
Comment 5 Simon Fraser (smfr) 2011-01-28 15:58:54 PST 
Test
Comment 6 Simon Fraser (smfr) 2011-01-28 17:49:09 PST 
Created attachment 80531 [details]
Need this change too.
Comment 7 Simon Fraser (smfr) 2011-01-28 21:02:55 PST 
http://trac.webkit.org/changeset/77045