Last Comment Bug 744901 - Store the page size in FrameMetrics in CSS pixels in addition to device pixels.
: Store the page size in FrameMetrics in CSS pixels in addition to device pixels.
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla14
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
Depends on: 749953 753762
Blocks: 742134 744916
  Show dependency treegraph
 
Reported: 2012-04-12 12:52 PDT by Jeff Muizelaar [:jrmuizel]
Modified: 2012-05-10 10:08 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta+


Attachments
Store the page size in FrameMetrics in CSS pixels in addition to device pixels (4.45 KB, patch)
2012-04-12 12:53 PDT, Jeff Muizelaar [:jrmuizel]
cjones.bugs: review+
Details | Diff | Splinter Review
typo's fixed (4.61 KB, patch)
2012-04-17 20:21 PDT, Jeff Muizelaar [:jrmuizel]
jmuizelaar: review+
Details | Diff | Splinter Review

Description Jeff Muizelaar [:jrmuizel] 2012-04-12 12:52:12 PDT
Some of the consumers (The Java frontend and RenderFrameParent) need the value in CSS pixels and using the device pixels gives slightly incorrect results because of rounding.
Comment 1 Jeff Muizelaar [:jrmuizel] 2012-04-12 12:53:44 PDT
Created attachment 614514 [details] [diff] [review]
Store the page size in FrameMetrics in CSS pixels in addition to device pixels
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-04-13 07:59:49 PDT
Comment on attachment 614514 [details] [diff] [review]
Store the page size in FrameMetrics in CSS pixels in addition to device pixels

What consumer wants CSS-pixel values, and how do they fix rounding errors?  This change on its own is fine, but I'm skeptical that it would legitimately fix anything.
Comment 3 Jeff Muizelaar [:jrmuizel] 2012-04-14 10:24:11 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> Comment on attachment 614514 [details] [diff] [review]
> Store the page size in FrameMetrics in CSS pixels in addition to device
> pixels
> 
> What consumer wants CSS-pixel values, and how do they fix rounding errors? 
> This change on its own is fine, but I'm skeptical that it would legitimately
> fix anything.

The consumer that this patch fixes is RenderFrameParent. It seems to currently assume that rounding doesn't happen.

The Java front end currently uses device pixel page sizes to calculate new device pixel page size which is obviously wrong. This patch is a step toward fixing that.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-04-16 18:31:08 PDT
Comment on attachment 614514 [details] [diff] [review]
Store the page size in FrameMetrics in CSS pixels in addition to device pixels

>diff --git a/gfx/layers/Layers.h b/gfx/layers/Layers.h

>+  // Consumer's often want to know the size before scaling to pixels
>+  // so we record this size as well.
>+  gfx::Size mCSSContentSize;
>+

I think this member is pretty self-explanatory, not sure this comment
is helpful.  (And it has a typo.)  Would drop it, up to you.
Comment 5 JP Rosevear [:jpr] 2012-04-17 12:44:27 PDT
Please do a complete risk assessment of all three pieces before landing, it seems like we could break a lot here.
Comment 6 Jeff Muizelaar [:jrmuizel] 2012-04-17 12:49:27 PDT
This patch has very minimal risk, it adds some fields and uses them in one consumer in a place that was previously incorrect.
Comment 7 Jeff Muizelaar [:jrmuizel] 2012-04-17 20:21:31 PDT
Created attachment 615998 [details] [diff] [review]
typo's fixed
Comment 8 Joe Drew (not getting mail) 2012-04-20 13:09:38 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9053acaa5e1 with the typos actually fixe'd.
Comment 9 Phil Ringnalda (:philor) 2012-04-20 20:04:11 PDT
https://hg.mozilla.org/mozilla-central/rev/e9053acaa5e1

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