Bug 196339

Summary: [WinCairo][WK2] WebView should call WebPageProxy::setIntrinsicDeviceScaleFactor for high DPI
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, koivisto, pvollan, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201361
https://bugs.webkit.org/show_bug.cgi?id=173152
Bug Depends on: 196340    
Bug Blocks: 201361    
Attachments:
Description Flags
WIP patch
none
WIP patch
none
WIP patch
none
WIP patch
none
WIP patch
none
WIP patch
none
WIP patch none

Fujii Hironori
Reported 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.
Attachments
WIP patch (5.42 KB, patch)
2019-03-27 22:13 PDT, Fujii Hironori
no flags
WIP patch (6.92 KB, patch)
2019-09-01 23:34 PDT, Fujii Hironori
no flags
WIP patch (11.16 KB, patch)
2019-09-02 00:35 PDT, Fujii Hironori
no flags
WIP patch (11.77 KB, patch)
2019-09-02 01:16 PDT, Fujii Hironori
no flags
WIP patch (12.52 KB, patch)
2019-09-02 03:03 PDT, Fujii Hironori
no flags
WIP patch (12.76 KB, patch)
2019-09-03 01:06 PDT, Fujii Hironori
no flags
WIP patch (24.82 KB, patch)
2022-01-31 18:17 PST, Fujii Hironori
no flags
Fujii Hironori
Comment 1 2019-03-27 22:13:22 PDT
Created attachment 366152 [details] WIP patch
Fujii Hironori
Comment 2 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.
Fujii Hironori
Comment 3 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.
Brent Fulgham
Comment 4 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.
Fujii Hironori
Comment 5 2019-09-01 23:34:01 PDT
Created attachment 377841 [details] WIP patch * Rebased onto ToT * points in all mouse events should be transformed
Fujii Hironori
Comment 6 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
Fujii Hironori
Comment 7 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
Fujii Hironori
Comment 8 2019-09-02 00:54:54 PDT
* Pink noisy lines by scrolling. (Same with WK1) https://ibb.co/y5xcg02
Fujii Hironori
Comment 9 2019-09-02 01:16:15 PDT
Created attachment 377844 [details] WIP patch
Fujii Hironori
Comment 10 2019-09-02 03:03:12 PDT
Created attachment 377847 [details] WIP patch
Fujii Hironori
Comment 11 2019-09-02 03:58:56 PDT
Fujii Hironori
Comment 12 2019-09-03 01:06:22 PDT
Created attachment 377874 [details] WIP patch
Fujii Hironori
Comment 13 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?
Brent Fulgham
Comment 14 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.
Fujii Hironori
Comment 15 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.
Fujii Hironori
Comment 16 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]
Fujii Hironori
Comment 17 2022-01-31 18:17:35 PST
Created attachment 450489 [details] WIP patch
ryosei.otaka
Comment 18 2024-05-21 01:11:42 PDT
EWS
Comment 19 2024-06-06 22:56:13 PDT
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
Comment 20 2024-06-06 22:57:16 PDT
Note You need to log in before you can comment on or make changes to this bug.