[webkit-reviews] review granted: [Bug 132895] Subpixel layout: Change Element.offset* client* scroll* return type to double. : [Attachment 231458] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Zalan Bujtas
<zalan at apple.com>'s request for review:
Bug 132895: Subpixel layout: Change Element.offset* client* scroll* return type
to double.
https://bugs.webkit.org/show_bug.cgi?id=132895

Attachment 231458: Patch
https://bugs.webkit.org/attachment.cgi?id=231458&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231458&action=review


> Source/WebCore/ChangeLog:14
> +	   subpixelCSSOMElementMetricsEnabled setting is added to be able to
turn this feature on/off
> +	   from Safari's debug menu. It toggles the return value from subpixel
to floored integral.

Not just from mainFrameView.contentsToRootView, by via WK2 prefs too,

> Source/WebCore/dom/Element.cpp:649
>      float zoomFactor = localZoomForRenderer(renderer);

Use a double to avoid float -> double promotion at all the / zoomFactor sites?

> Source/WebCore/dom/Element.cpp:749
> +	   return
convertToNonSubpixelValueIfNeeded(adjustLayoutUnitForAbsoluteZoom(renderer->cli
entTop(), *renderer).toDouble(), renderer->document());
> +#else
> +	   return adjustForAbsoluteZoom(renderer->clientTop(), *renderer);

Do we need both kinds of adjust*forAbsoluteZoom?

> Source/WebCore/page/Settings.in:197
> +subpixelCSSOMElementMetricsEnabled initial=true

I wonder if we should expose this through WebKit1 pref too?