WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 69739
Loss of precision when converting from double to int and double to float in FrameView::zoomAnimatorTransformChanged()
https://bugs.webkit.org/show_bug.cgi?id=69739
Summary
Loss of precision when converting from double to int and double to float in F...
Daniel Bates
Reported
2011-10-09 21:48:23 PDT
Bug #68035
(
http://trac.webkit.org/changeset/97034
) added the method FrameView::zoomAnimatorTransformChanged() which implicitly converts a double precision computation (64-bit) to an integer result (32-bit) and hence causes a warning on the Leopard Intel Debug bot: [[ cc1plus: warnings being treated as errors /Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/Source/WebCore/page/FrameView.cpp: In member function 'virtual void WebCore::FrameView::zoomAnimatorTransformChanged(double, double, double, WebCore::ScrollableArea::ZoomAnimationState)': /Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/Source/WebCore/page/FrameView.cpp:1230: warning: implicit conversion shortens 64-bit value into a 32-bit value /Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/Source/WebCore/page/FrameView.cpp:1230: warning: implicit conversion shortens 64-bit value into a 32-bit value /Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/Source/WebCore/page/FrameView.cpp:1230: warning: implicit conversion shortens 64-bit value into a 32-bit value ]] (
http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Build%29/builds/40404/steps/compile-webkit/logs/stdio
)
Attachments
Patch
(4.25 KB, patch)
2011-10-11 05:56 PDT
,
W. James MacLean
no flags
Details
Formatted Diff
Diff
Patch
(5.86 KB, patch)
2011-10-11 06:07 PDT
,
W. James MacLean
no flags
Details
Formatted Diff
Diff
Patch
(6.12 KB, patch)
2011-10-11 08:56 PDT
,
W. James MacLean
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2011-10-09 21:56:11 PDT
I am unsure what's the best way to fix this issue at this time. For now, I explicitly converted the double precision results to an integers and landed this in <
http://trac.webkit.org/changeset/97041
>. We need to look into this some more.
Daniel Bates
Comment 2
2011-10-09 22:23:48 PDT
Explicitly converted double precision result to single precision result and committed fix in <
http://trac.webkit.org/changeset/97042
>
Daniel Bates
Comment 3
2011-10-09 23:14:42 PDT
From reading the patch (
attachment #110142
[details]
) the only caller of zoomAnimatorTransformChanged() passes floats for the arguments. So, it seems sufficient to modify the prototype of zoomAnimatorTransformChanged() to take floats instead of doubles. Then we can remove these casts. Is there a reason that zoomAnimatorTransformChanged() takes doubles?
Simon Fraser (smfr)
Comment 4
2011-10-10 11:59:58 PDT
Who added it?
W. James MacLean
Comment 5
2011-10-11 05:56:51 PDT
Created
attachment 110504
[details]
Patch
W. James MacLean
Comment 6
2011-10-11 05:58:13 PDT
(In reply to
comment #4
)
> Who added it?
I did. I have uploaded a patch to amend this.
W. James MacLean
Comment 7
2011-10-11 06:07:28 PDT
Created
attachment 110505
[details]
Patch
W. James MacLean
Comment 8
2011-10-11 06:08:39 PDT
Revised to include BuiltInPDFView.h in WebKit2.
Simon Fraser (smfr)
Comment 9
2011-10-11 08:43:50 PDT
Comment on
attachment 110505
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=110505&action=review
> Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). > + > + Test coverage provided by existing zoom-animator tests.
Please say what the patch does here.
W. James MacLean
Comment 10
2011-10-11 08:56:11 PDT
Created
attachment 110518
[details]
Patch
W. James MacLean
Comment 11
2011-10-11 08:58:07 PDT
(In reply to
comment #10
)
> Created an attachment (id=110518) [details] > Patch
Revised changelog comments as per your suggestion ... let me know if they look OK.(In reply to
comment #9
)
> (From update of
attachment 110505
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=110505&action=review
> > > Source/WebCore/ChangeLog:8 > > + Reviewed by NOBODY (OOPS!). > > + > > + Test coverage provided by existing zoom-animator tests. > > Please say what the patch does here.
Revised changelog comments as per your suggestion ... let me know if they look OK.
WebKit Review Bot
Comment 12
2011-10-11 15:43:01 PDT
Comment on
attachment 110518
[details]
Patch Clearing flags on attachment: 110518 Committed
r97188
: <
http://trac.webkit.org/changeset/97188
>
WebKit Review Bot
Comment 13
2011-10-11 15:43:06 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug