Bug 196339 - [WinCairo][WK2] WebView should call WebPageProxy::setIntrinsicDeviceScaleFactor for high DPI
Summary: [WinCairo][WK2] WebView should call WebPageProxy::setIntrinsicDeviceScaleFact...
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 196340
Blocks: 201361
  Show dependency treegraph
 
Reported: 2019-03-27 22:12 PDT by Fujii Hironori
Modified: 2019-09-10 00:23 PDT (History)
4 users (show)

See Also:


Attachments
WIP patch (5.42 KB, patch)
2019-03-27 22:13 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (6.92 KB, patch)
2019-09-01 23:34 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (11.16 KB, patch)
2019-09-02 00:35 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (11.77 KB, patch)
2019-09-02 01:16 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (12.52 KB, patch)
2019-09-02 03:03 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
WIP patch (12.76 KB, patch)
2019-09-03 01:06 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2019-03-27 22:12:36 PDT
[WinCairo][WK2] WebView should call WebPageProxy::setIntrinsicDeviceScaleFactor for high DPI

WebView paints tiny texts in high DPI at the moment.
Comment 1 Fujii Hironori 2019-03-27 22:13:22 PDT
Created attachment 366152 [details]
WIP patch
Comment 2 Fujii Hironori 2019-03-29 00:17:29 PDT
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.
Comment 3 Fujii Hironori 2019-09-01 22:26:11 PDT
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.
Comment 4 Brent Fulgham 2019-09-01 22:42:18 PDT
Comment on attachment 366152 [details]
WIP patch

This looks reasonable to me. If EWS is happy I suggest landing.
Comment 5 Fujii Hironori 2019-09-01 23:34:01 PDT
Created attachment 377841 [details]
WIP patch

* Rebased onto ToT
* points in all mouse events should be transformed
Comment 6 Fujii Hironori 2019-09-02 00:35:08 PDT
* 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
Comment 7 Fujii Hironori 2019-09-02 00:35:30 PDT
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
Comment 8 Fujii Hironori 2019-09-02 00:54:54 PDT
* Pink noisy lines by scrolling. (Same with WK1) https://ibb.co/y5xcg02
Comment 9 Fujii Hironori 2019-09-02 01:16:15 PDT
Created attachment 377844 [details]
WIP patch
Comment 10 Fujii Hironori 2019-09-02 03:03:12 PDT
Created attachment 377847 [details]
WIP patch
Comment 11 Fujii Hironori 2019-09-02 03:58:56 PDT
* 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
Comment 12 Fujii Hironori 2019-09-03 01:06:22 PDT
Created attachment 377874 [details]
WIP patch
Comment 13 Fujii Hironori 2019-09-03 03:08:36 PDT
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?
Comment 14 Brent Fulgham 2019-09-03 08:56:35 PDT
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.
Comment 15 Fujii Hironori 2019-09-05 20:39:00 PDT
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.
Comment 16 Fujii Hironori 2019-09-10 00:23:19 PDT
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]