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]
:
: Jet Villegas (:jet)
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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Jeff Muizelaar [:jrmuizel] 2012-04-17 20:21:31 PDT
Created attachment 615998 [details] [diff] [review]
typo's fixed
Comment 8 User image 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 User image 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.