Last Comment Bug 670461 - Fix conversion from 'double' to 'PRUint32' build warning in gfx/src/nsRenderingContext.h(73)
: Fix conversion from 'double' to 'PRUint32' build warning in gfx/src/nsRenderi...
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla9
Assigned To: Ed Morley [:emorley]
: Milan Sreckovic [:milan]
Depends on: 670338
Blocks: buildwarning
  Show dependency treegraph
Reported: 2011-07-09 12:38 PDT by Ed Morley [:emorley]
Modified: 2011-08-19 03:08 PDT (History)
3 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (1.08 KB, patch)
2011-07-09 12:46 PDT, Ed Morley [:emorley]
joe: review+
Details | Diff | Splinter Review
Patch v2 (1.25 KB, patch)
2011-07-16 06:23 PDT, Ed Morley [:emorley]
joe: review-
Details | Diff | Splinter Review
Patch for checkin (1.08 KB, patch)
2011-08-15 15:56 PDT, Ed Morley [:emorley]
no flags Details | Diff | Splinter Review

Description User image Ed Morley [:emorley] 2011-07-09 12:38:29 PDT
Shows up 144 times in my MSVC2010 local build...

/dist/include/nsRenderingContext.h(73) : warning C4244: 'return' : conversion from 'double' to 'PRInt32', possible loss of data
Comment 1 User image Ed Morley [:emorley] 2011-07-09 12:46:35 PDT
Created attachment 545014 [details] [diff] [review]
Patch v1

Replies upon the double overload being added in bug 670338.

Thanks :-)
Comment 2 User image Ed Morley [:emorley] 2011-07-09 14:35:27 PDT
Comment 3 User image Joe Drew (not getting mail) 2011-07-12 12:30:49 PDT
Comment on attachment 545014 [details] [diff] [review]
Patch v1

Why is mP2A stored as a double if we only use it as a PRInt32?

(If there's a good reason, please rerequest review on this patch.)
Comment 4 User image Ed Morley [:emorley] 2011-07-16 03:36:53 PDT
I have no idea (and had thought the same myself, but presumed I was just missing something).

So would you like me to just change it to PRInt32?
Comment 5 User image Ed Morley [:emorley] 2011-07-16 06:23:30 PDT
Created attachment 546317 [details] [diff] [review]
Patch v2

Presuming the answer to comment 5 is yes, this updated patch stores mP2A as PRInt32 rather than a double, silencing the build warning.
Comment 6 User image Ed Morley [:emorley] 2011-07-16 09:57:36 PDT
Comment 7 User image Ed Morley [:emorley] 2011-07-16 15:37:07 PDT
(In reply to comment #6)

Failed try, by causing reftest failures on all platforms along the lines of:

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/reftest/tests/layout/reftests/pixel-rounding/iframe-1.html | image comparison (==)

Comment 8 User image Joe Drew (not getting mail) 2011-07-20 14:27:39 PDT
Comment on attachment 546317 [details] [diff] [review]
Patch v2

Given try results, clearly there's a reason!
Comment 9 User image Ed Morley [:emorley] 2011-08-02 07:49:52 PDT
Joe, I may have just messed up the v2 patch, given there are other instances where the variable mP2A are used (not all in scope admittedly):

Could you take a quick look before we resort to casting, in case changing mP2A to PRInt32 is still a better approach?

Thanks! :-)
Comment 10 User image Ed Morley [:emorley] 2011-08-14 03:10:13 PDT
Joe, any ideas about comment 9? Thanks :-)
Comment 11 User image Joe Drew (not getting mail) 2011-08-15 13:21:14 PDT
Ah, yes - you're clearly right that changing the type is wrong, since it's used internally in nsRenderingContext as a double. Your first patch is therefore correct!
Comment 12 User image Ed Morley [:emorley] 2011-08-15 15:56:37 PDT
Created attachment 553293 [details] [diff] [review]
Patch for checkin

Great, thanks Joe :-)

Patch for checkin based on the v1 approach, but updated to tip to take account of bug 678222 landing (which changed the return type of AppUnitsPerDevPixel() to be PRUint32).
Comment 14 User image Marco Bonardo [::mak] 2011-08-19 03:08:06 PDT

Note You need to log in before you can comment on or make changes to this bug.