805761 - FrameMetrics.h(89) : warning C4244: 'argument' : conversion from 'gfxFloat' to 'mozilla::gfx::Float', possible loss of data

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I can see a lot of this warning with VC.
FrameMetrics.h(89) : warning C4244: 'argument' : conversion from 'gfxFloat' to 'mozilla::gfx::Float', possible loss of data

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

Comment 2

[Masatoshi Kimura [:emk]]

Comment on attachment 675500 [details] [diff] [review]
Patch

>+      mScrollOffset.x *
>+        static_cast<gfx::Float>(LayersPixelsPerCSSPixel().width),
>+      mScrollOffset.y *
>+        static_cast<gfx::Float>(LayersPixelsPerCSSPixel().height));
Is it OK to lose precision before multiplying?

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't know the resolution of them.

Comment 4

[Masatoshi Kimura [:emk]]

Doesn't
  static_cast<gfx::Float>(mScrollOffset.x * LayersPixelsPerCSSPixel().width),
  static_cast<gfx::Float>(mScrollOffset.y * LayersPixelsPerCSSPixel().height)
work?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

still building ;-)

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Alternative Patch

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=de411568f98a

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 675500 [details] [diff] [review]
Patch

Both patches work fine, so, the other patch should be used for supporting higher resolution.

Comment 8

[Doug Sherk (:drs) (inactive)]

Comment on attachment 675529 [details] [diff] [review]
Alternative Patch

Review of attachment 675529 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/FrameMetrics.h
@@ +86,5 @@
>    gfx::Point GetScrollOffsetInLayerPixels() const
>    {
> +    return gfx::Point(
> +      static_cast<gfx::Float>(mScrollOffset.x *
> +                                LayersPixelsPerCSSPixel().width),

Put this on one line please, like this:
  static_cast<gfx::Float>(
    mScrollOffset.x * LayersPixelsPerCSSPixel().width),

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd6b5dcd73c3

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dd6b5dcd73c3