Bug 196339 – [WinCairo][WK2] WebView should call WebPageProxy::setIntrinsicDeviceScaleFactor for high DPI

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Fujii Hironori]

[WinCairo][WK2] WebView should call WebPageProxy::setIntrinsicDeviceScaleFactor for high DPI

WebView paints tiny texts in high DPI at the moment.

[Fujii Hironori]

Created attachment 366152 [details]
WIP patch

[Fujii Hironori]

In this approach, we need to convert between Windows coordinate system and WebCore coordinate system in so many place.
We can know how much it is by grepping 'deviceScaleFactor' in WebKitLegacy Windows port.
This approach seems too complicated for Windows. I filed Bug 196391 for another approach.

[Fujii Hironori]

It turned out that there are some problem with Bug 196391 approach.
1. scrool bars aren't scaled
2. web inspector isn't scaled

Reopen this. We should implement the same approach with WK1 Windows port.

[Brent Fulgham]

Comment on attachment 366152 [details]
WIP patch

This looks reasonable to me. If EWS is happy I suggest landing.

[Fujii Hironori]

Created attachment 377841 [details]
WIP patch

* Rebased onto ToT
* points in all mouse events should be transformed

[Fujii Hironori]

* Transformed points in all mouse events
* Transformed web inspector geometry

Issues:

* Caret doesn't blink. 
* Web inspector renders texts sparsely (same with WebKit1) https://ibb.co/G9XNP9J

[Fujii Hironori]

Created attachment 377843 [details]
WIP patch

* Transformed points in all mouse events
* Transformed web inspector geometry

* Caret doesn't blink. Why?
* Web inspector renders texts sparsely (same with WebKit1) https://ibb.co/G9XNP9J

[Fujii Hironori]

* Pink noisy lines by scrolling. (Same with WK1) https://ibb.co/y5xcg02

[Fujii Hironori]

Created attachment 377844 [details]
WIP patch

[Fujii Hironori]

Created attachment 377847 [details]
WIP patch

[Fujii Hironori]

* MouseEvent.screenX and MouseEvent.screenY should be in CSS pixels.

https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/screenX
https://drafts.csswg.org/cssom-view/#web-exposed-screen-area

[Fujii Hironori]

Created attachment 377874 [details]
WIP patch

[Fujii Hironori]

Windows supports fractional device scale factors. Actually, I'm
using 1.5x device scale factor display.

It seems that WebKit has architectural problem for fractional
device scale factors.

In 1.5x DPI, 2x2 CSS pixels are rendered onto 3x3 device pixels.
If WebKit is going to render some text in (1,1) CSS pixel position,
it will be placed in (1.5, 1.5) device pixel position.

Here is test case to demonstrate the problem:
https://jsfiddle.net/fujihiro/2wa4zbhy/1/embedded/result/

At first rendering, all text are shown clear:
https://ibb.co/dthYf8X

But, after repaint texts by hovering all texts, only odd-y-axis positioned texts are blurred and have noisy horizontal lines:
https://ibb.co/ZWncf6z

Brent, do you have a fractional device scale factor display?

[Brent Fulgham]

Fujii -- I think this is indeed a problem. Apple devices are locked in at 1x, 2x, and 3x dimensions, so we have not had to deal with this.

I'll pull Zalan and Antti in for discussion.

[Fujii Hironori]

Windows WebKit1 also has the fractional scaling factor issue. It repaints the whole scroll area of WebChromeClient::scroll.

Bug 173152 – [Win][HighDPI] Repaint glitches when scrolling.
Bug 193542 – [Win] Scrolling should not repaint dirty region.

[Fujii Hironori]

I compiled and checked ToT Chromium.
It is using page zoom for device scale factor.
page_zoom_factor is 1.5 in LocalFrame::SetPageAndTextZoomFactors in 1.5 device scale factor display.

Callstack:

> blink_core.dll!blink::LocalFrame::SetPageAndTextZoomFactors(float page_zoom_factor, float text_zoom_factor) Line 768	C++
> blink_core.dll!blink::LocalFrame::SetPageZoomFactor(float factor) Line 740	C++
> blink_core.dll!blink::WebViewImpl::PropagateZoomFactorToLocalFrameRoots(blink::Frame * frame, float zoom_factor) Line 2362	C++
> blink_core.dll!blink::WebViewImpl::SetZoomLevel(double zoom_level) Line 2395	C++
> blink_core.dll!blink::WebViewImpl::SetZoomFactorForDeviceScaleFactor(float zoom_factor_for_device_scale_factor) Line 2493	C++
> content.dll!content::RenderWidget::UpdateWebViewWithDeviceScaleFactor() Line 1969	C++
> content.dll!content::RenderWidget::UpdateSurfaceAndScreenInfo(const viz::LocalSurfaceIdAllocation & new_local_surface_id_allocation, const gfx::Rect & compositor_viewport_pixel_rect, const content::ScreenInfo & new_screen_info) Line 2238	C++
> content.dll!content::RenderWidget::SynchronizeVisualProperties(const content::VisualProperties & visual_properties) Line 1654	C++
> content.dll!content::RenderWidget::OnSynchronizeVisualProperties(const content::VisualProperties & visual_properties_from_browser) Line 896	C++
> content.dll!base::DispatchToMethodImpl<content::RenderWidget *,void (content::RenderWidget::*)(const content::VisualProperties &),std::__1::tuple<content::VisualProperties>,0>(content::RenderWidget * const & obj, void(content::RenderWidget::*)(const content::VisualProperties &) method, std::__1::tuple<content::VisualProperties> && args, std::__1::integer_sequence<unsigned long long,0>) Line 53	C++
> content.dll!base::DispatchToMethod<content::RenderWidget *,void (content::RenderWidget::*)(const content::VisualProperties &),std::__1::tuple<content::VisualProperties> >(content::RenderWidget * const & obj, void(content::RenderWidget::*)(const content::VisualProperties &) method, std::__1::tuple<content::VisualProperties> && args) Line 60	C++
> content.dll!IPC::DispatchToMethod<content::RenderWidget,void (content::RenderWidget::*)(const content::VisualProperties &),void,std::__1::tuple<content::VisualProperties> >(content::RenderWidget * obj, void(content::RenderWidget::*)(const content::VisualProperties &) method, void *, std::__1::tuple<content::VisualProperties> && tuple) Line 51	C++
> content.dll!IPC::MessageT<WidgetMsg_SynchronizeVisualProperties_Meta,std::__1::tuple<content::VisualProperties>,void>::Dispatch<content::RenderWidget,content::RenderWidget,void,void (content::RenderWidget::*)(const content::VisualProperties &)>(const IPC::Message * msg, content::RenderWidget * obj, content::RenderWidget * sender, void * parameter, void(content::RenderWidget::*)(const content::VisualProperties &) func) Line 147	C++
> content.dll!content::RenderWidget::OnMessageReceived(const IPC::Message & message) Line 622	C++
> ipc.dll!IPC::MessageRouter::RouteMessage(const IPC::Message & msg) Line 56	C++
> content.dll!content::ChildThreadImpl::ChildThreadMessageRouter::RouteMessage(const IPC::Message & msg) Line 502	C++
> ipc.dll!IPC::MessageRouter::OnMessageReceived(const IPC::Message & msg) Line 48	C++
> content.dll!content::ChildThreadImpl::OnMessageReceived(const IPC::Message & msg) Line 841	C++
> ipc.dll!IPC::ChannelProxy::Context::OnDispatchMessage(const IPC::Message & message) Line 324	C++
> ipc.dll!base::internal::FunctorTraits<void (IPC::ChannelProxy::Context::*)(const IPC::Message &),void>::Invoke<void (IPC::ChannelProxy::Context::*)(const IPC::Message &),scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message>(void(IPC::ChannelProxy::Context::*)(const IPC::Message &) method, scoped_refptr<IPC::ChannelProxy::Context> && receiver_ptr, IPC::Message && args) Line 499	C++
> ipc.dll!base::internal::InvokeHelper<0,void>::MakeItSo<void (IPC::ChannelProxy::Context::*)(const IPC::Message &),scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message>(void(IPC::ChannelProxy::Context::*)(const IPC::Message &) && functor, scoped_refptr<IPC::ChannelProxy::Context> && args, IPC::Message && args) Line 599	C++
> ipc.dll!base::internal::Invoker<base::internal::BindState<void (IPC::ChannelProxy::Context::*)(const IPC::Message &),scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message>,void ()>::RunImpl<void (IPC::ChannelProxy::Context::*)(const IPC::Message &),std::__1::tuple<scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message>,0,1>(void(IPC::ChannelProxy::Context::*)(const IPC::Message &) && functor, std::__1::tuple<scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message> && bound, std::__1::integer_sequence<unsigned long long,0,1>) Line 672	C++
> ipc.dll!base::internal::Invoker<base::internal::BindState<void (IPC::ChannelProxy::Context::*)(const IPC::Message &),scoped_refptr<IPC::ChannelProxy::Context>,IPC::Message>,void ()>::RunOnce(base::internal::BindStateBase * base) Line 641	C++
> base.dll!base::OnceCallback<void ()>::Run() Line 99	C++
> base.dll!base::TaskAnnotator::RunTask(const char * trace_event_name, base::PendingTask * pending_task) Line 144	C++
> base.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::sequence_manager::LazyNow * continuation_lazy_now, bool * ran_task) Line 366	C++
> base.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoSomeWork() Line 221	C++
> base.dll!base::MessagePumpDefault::Run(base::MessagePump::Delegate * delegate) Line 40	C++
> base.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool application_tasks_allowed, base::TimeDelta timeout) Line 467	C++
> base.dll!base::RunLoop::Run() Line 156	C++
> content.dll!content::RendererMain(const content::MainFunctionParams & parameters) Line 214	C++
> content.dll!content::RunOtherNamedProcessTypeMain(const std::__1::basic_string<char,std::__1::char_traits<char>,std::__1::allocator<char> > & process_type, const content::MainFunctionParams & main_function_params, content::ContentMainDelegate * delegate) Line 581	C++
> content.dll!content::ContentMainRunnerImpl::Run(bool start_service_manager_only) Line 874	C++
> content.dll!content::ContentServiceManagerMainDelegate::RunEmbedderProcess() Line 52	C++
> embedder.dll!service_manager::Main(const service_manager::MainParams & params) Line 423	C++
> content.dll!content::ContentMain(const content::ContentMainParams & params) Line 20	C++
> chrome.dll!ChromeMain(HINSTANCE__ * instance, sandbox::SandboxInterfaceInfo * sandbox_info, __int64 exe_entry_point_ticks) Line 110	C++
> chrome.exe!MainDllLoader::Launch(HINSTANCE__ * instance, base::TimeTicks exe_entry_point_ticks) Line 202	C++
> chrome.exe!wWinMain(HINSTANCE__ * instance, HINSTANCE__ * prev, wchar_t *, int) Line 234	C++
> [External Code]

[Fujii Hironori]

Created attachment 450489 [details]
WIP patch

Pull request: https://github.com/WebKit/WebKit/pull/28842

[EWS]

Committed 279794@main (b6d35ab492c2): <https://commits.webkit.org/279794@main>

Reviewed commits have been landed. Closing PR #28842 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/129382860>