Bug 201361

Summary: [WinCairo, FTW] Properly handle device scale factor
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: REOPENED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, don.olmstead, fujii.hironori, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 10   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=196339
Bug Depends on: 196339, 196340    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Wrong Patch none

Brent Fulgham
Reported 2019-08-30 15:38:09 PDT
The Modern WebKit implementation does not handle device scale factor properly. The WebContent process runs without DPI awareness and draws everything into 1x buffers, even when the system is running at higher DPI. This leads to Windows scaling the content and lower resolution output. This patch makes a few corrections: 1. It calls "SetProcessDPIAware" at launch in the WebContent process so that it can make appropriate measurements of the screen resolution. 2. We update the WebPage the proper "setIntrinsicDeviceScaleFactor" value based on screen resolution so that it can pass it to the WebContent process for rendering calculations. 3. We do call WKPageSetPageZoomFactor with the screen DPI scale factor, since this is meant to be a logical screen zoom, not a correction for high DPI.
Attachments
Patch (8.05 KB, patch)
2019-08-30 23:56 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Wrong Patch (10.50 KB, patch)
2019-09-01 21:20 PDT, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2019-08-30 23:32:32 PDT
The attached patch gets things working properly so that Windows controls are drawn properly. It does not handle changes in DPI (such as adjusting the Windows settings, or dragging a WebView from a high DPI monitor to a low-DPI monitor.
Brent Fulgham
Comment 2 2019-08-30 23:56:08 PDT
Created attachment 377787 [details] Patch
WebKit Commit Bot
Comment 3 2019-09-01 10:26:48 PDT
Comment on attachment 377787 [details] Patch Rejecting attachment 377787 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 377787, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebKit/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/12989465
Brent Fulgham
Comment 4 2019-09-01 11:13:04 PDT
Committed r249366: <https://trac.webkit.org/changeset/249366>
Radar WebKit Bug Importer
Comment 5 2019-09-01 11:14:14 PDT
<rdar://problem/54932919>
Fujii Hironori
Comment 6 2019-09-01 19:09:16 PDT
device scale factor is not correct in WinCairo WK2 WinCairo WK2 crashes in some web sites, for example https://webkit.org and https://www.youtube.com Callstack: > cairo.dll!_pixman_implementation_create_sse2() C > cairo.dll!_pixman_gradient_walker_write_wide() C > cairo.dll!pixman_image_composite32() C > cairo.dll!_inplace_src_spans(void * abstract_renderer, int y, int h, const _cairo_half_open_span * spans, unsigned int num_spans) Line 2716 C > cairo.dll!generate_row(_cairo_span_renderer * renderer, const _rectangle * r, int y, int h, unsigned short coverage) Line 626 C > cairo.dll!_cairo_rectangular_scan_converter_generate(void * converter, _cairo_span_renderer * renderer) Line 673 C > cairo.dll!composite_boxes(const cairo_spans_compositor * compositor, _cairo_composite_rectangles * extents, _cairo_boxes_t * boxes) Line 741 C > cairo.dll!clip_and_composite_boxes(const cairo_spans_compositor * compositor, _cairo_composite_rectangles * extents, _cairo_boxes_t * boxes) Line 888 C > cairo.dll!_cairo_spans_compositor_mask(const cairo_compositor * _compositor, _cairo_composite_rectangles * extents) Line 1000 C > cairo.dll!_cairo_compositor_paint(const cairo_compositor * compositor, _cairo_surface * surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 67 C > cairo.dll!_cairo_image_surface_paint(void * abstract_surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 931 C > cairo.dll!_cairo_surface_paint(_cairo_surface * surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 2199 C > cairo.dll!_cairo_gstate_paint(_cairo_gstate * gstate) Line 1061 C > cairo.dll!_cairo_default_context_paint_with_alpha(void * abstract_cr, double alpha) Line 971 C > cairo.dll!cairo_paint_with_alpha(_cairo * cr, double alpha) Line 2248 C > WebKit2.dll!WebCore::Cairo::drawPatternToCairoContext(_cairo * cr, _cairo_pattern * pattern, const WebCore::FloatRect & destRect, float alpha) Line 156 C++ > WebKit2.dll!WebCore::Cairo::drawSurface(WebCore::PlatformContextCairo & platformContext, _cairo_surface * surface, const WebCore::FloatRect & destRect, const WebCore::FloatRect & originalSrcRect, WebCore::InterpolationQuality imageInterpolationQuality, float globalAlpha, const WebCore::Cairo::ShadowState & shadowState) Line 988 C++ > WebKit2.dll!WebKit::ShareableBitmap::paint(WebCore::GraphicsContext & context, float scaleFactor, const WebCore::IntPoint & dstPoint, const WebCore::IntRect & srcRect) Line 80 C++ > WebKit2.dll!WebKit::BackingStore::incorporateUpdate(WebKit::ShareableBitmap * bitmap, const WebKit::UpdateInfo & updateInfo) Line 93 C++ > WebKit2.dll!WebKit::BackingStore::incorporateUpdate(const WebKit::UpdateInfo & updateInfo) Line 67 C++ > WebKit2.dll!WebKit::DrawingAreaProxyCoordinatedGraphics::incorporateUpdate(const WebKit::UpdateInfo & updateInfo) Line 254 C++ > WebKit2.dll!WebKit::DrawingAreaProxyCoordinatedGraphics::update(unsigned __int64 backingStoreStateID, const WebKit::UpdateInfo & updateInfo) Line 164 C++ > WebKit2.dll!IPC::callMemberFunctionImpl<WebKit::DrawingAreaProxy,void (WebKit::DrawingAreaProxy::*)(unsigned long long, const WebKit::UpdateInfo &),std::tuple<unsigned long long,WebKit::UpdateInfo>,0,1>(WebKit::DrawingAreaProxy * object, void(WebKit::DrawingAreaProxy::*)(unsigned __int64, const WebKit::UpdateInfo &) function, std::tuple<unsigned long long,WebKit::UpdateInfo> && args, std::integer_sequence<unsigned long long,0,1>) Line 42 C++ > WebKit2.dll!IPC::callMemberFunction<WebKit::DrawingAreaProxy,void (WebKit::DrawingAreaProxy::*)(unsigned long long, const WebKit::UpdateInfo &),std::tuple<unsigned long long,WebKit::UpdateInfo>,std::integer_sequence<unsigned long long,0,1> >(std::tuple<unsigned long long,WebKit::UpdateInfo> && args, WebKit::DrawingAreaProxy * object, void(WebKit::DrawingAreaProxy::*)(unsigned __int64, const WebKit::UpdateInfo &) function) Line 47 C++ > WebKit2.dll!IPC::handleMessage<Messages::DrawingAreaProxy::Update,WebKit::DrawingAreaProxy,void (WebKit::DrawingAreaProxy::*)(unsigned long long, const WebKit::UpdateInfo &)>(IPC::Decoder & decoder, WebKit::DrawingAreaProxy * object, void(WebKit::DrawingAreaProxy::*)(unsigned __int64, const WebKit::UpdateInfo &) function) Line 121 C++ > WebKit2.dll!WebKit::DrawingAreaProxy::didReceiveMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 44 C++ > WebKit2.dll!IPC::MessageReceiverMap::dispatchMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 124 C++ > WebKit2.dll!WebKit::AuxiliaryProcessProxy::dispatchMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 155 C++ > WebKit2.dll!WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 662 C++ > WebKit2.dll!IPC::Connection::dispatchMessage(IPC::Decoder & decoder) Line 940 C++ > WebKit2.dll!IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder,std::default_delete<IPC::Decoder> > message) Line 993 C++ > WebKit2.dll!IPC::Connection::dispatchIncomingMessages() Line 1097 C++ > WebKit2.dll!IPC::Connection::enqueueIncomingMessage::<unnamed-tag>::operator()() Line 914 C++ > WebKit2.dll!WTF::Detail::CallableWrapper<`lambda at ..\..\Source\WebKit\Platform\IPC\Connection.cpp:912:30',void>::call() Line 52 C++ > WTF.dll!WTF::Function<void ()>::operator()() Line 79 C++ > WTF.dll!WTF::RunLoop::performWork() Line 108 C++ > WTF.dll!WTF::RunLoop::wndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 57 C++ > WTF.dll!WTF::RunLoop::RunLoopWndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 39 C++ > [External Code] > WebKit.dll!WebKitMessageLoop::run(HACCEL__ * hAccelTable) Line 94 C++ > MiniBrowserLib.dll!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 97 C++ > MiniBrowserLib.dll!dllLauncherEntryPoint(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 115 C++ > MiniBrowser.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 232 C++ > [External Code]
Fujii Hironori
Comment 7 2019-09-01 19:11:59 PDT
Reverted r249366 for reason: WinCairo WebKit2 crashes in some websites and the device scale factor is not correct in high DPI. Committed r249371: <https://trac.webkit.org/changeset/249371>
Fujii Hironori
Comment 8 2019-09-01 20:03:17 PDT
Callstack in Comment 6 have incorrect pixman function names for https://github.com/WebKitForWindows/WinCairoRequirements/issues/26 Here is the correct callstack: > [Inline Frame] cairo.dll!combine4(const __m128i *) Line 550 C > cairo.dll!sse2_combine_add_u(pixman_implementation_t * imp, pixman_op_t op, unsigned int * dst, const unsigned int * src, const unsigned int * mask, int width) Line 1380 C > cairo.dll!general_composite_rect(pixman_implementation_t * imp, pixman_composite_info_t * info) Line 225 C > cairo.dll!pixman_image_composite32(pixman_op_t op, pixman_image * src, pixman_image * mask, pixman_image * dest, int src_x, int src_y, int mask_x, int mask_y, int dest_x, int dest_y, int width, int height) Line 700 C > cairo.dll!_inplace_src_spans(void * abstract_renderer, int y, int h, const _cairo_half_open_span * spans, unsigned int num_spans) Line 2716 C > cairo.dll!generate_row(_cairo_span_renderer * renderer, const _rectangle * r, int y, int h, unsigned short coverage) Line 626 C > cairo.dll!_cairo_rectangular_scan_converter_generate(void * converter, _cairo_span_renderer * renderer) Line 673 C > cairo.dll!composite_boxes(const cairo_spans_compositor * compositor, _cairo_composite_rectangles * extents, _cairo_boxes_t * boxes) Line 741 C > cairo.dll!clip_and_composite_boxes(const cairo_spans_compositor * compositor, _cairo_composite_rectangles * extents, _cairo_boxes_t * boxes) Line 888 C > cairo.dll!_cairo_spans_compositor_mask(const cairo_compositor * _compositor, _cairo_composite_rectangles * extents) Line 1000 C > cairo.dll!_cairo_compositor_paint(const cairo_compositor * compositor, _cairo_surface * surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 67 C > cairo.dll!_cairo_image_surface_paint(void * abstract_surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 931 C > cairo.dll!_cairo_surface_paint(_cairo_surface * surface, _cairo_operator op, const _cairo_pattern * source, const _cairo_clip * clip) Line 2199 C > cairo.dll!_cairo_gstate_paint(_cairo_gstate * gstate) Line 1061 C > cairo.dll!_cairo_default_context_paint_with_alpha(void * abstract_cr, double alpha) Line 971 C > cairo.dll!cairo_paint_with_alpha(_cairo * cr, double alpha) Line 2248 C > WebKit2.dll!WebCore::Cairo::drawPatternToCairoContext(_cairo * cr, _cairo_pattern * pattern, const WebCore::FloatRect & destRect, float alpha) Line 156 C++ > WebKit2.dll!WebCore::Cairo::drawSurface(WebCore::PlatformContextCairo & platformContext, _cairo_surface * surface, const WebCore::FloatRect & destRect, const WebCore::FloatRect & originalSrcRect, WebCore::InterpolationQuality imageInterpolationQuality, float globalAlpha, const WebCore::Cairo::ShadowState & shadowState) Line 988 C++ > WebKit2.dll!WebKit::ShareableBitmap::paint(WebCore::GraphicsContext & context, float scaleFactor, const WebCore::IntPoint & dstPoint, const WebCore::IntRect & srcRect) Line 80 C++ > WebKit2.dll!WebKit::BackingStore::incorporateUpdate(WebKit::ShareableBitmap * bitmap, const WebKit::UpdateInfo & updateInfo) Line 93 C++ > WebKit2.dll!WebKit::BackingStore::incorporateUpdate(const WebKit::UpdateInfo & updateInfo) Line 67 C++ > WebKit2.dll!WebKit::DrawingAreaProxyCoordinatedGraphics::incorporateUpdate(const WebKit::UpdateInfo & updateInfo) Line 254 C++ > WebKit2.dll!WebKit::DrawingAreaProxyCoordinatedGraphics::update(unsigned __int64 backingStoreStateID, const WebKit::UpdateInfo & updateInfo) Line 164 C++ > WebKit2.dll!IPC::callMemberFunctionImpl<WebKit::DrawingAreaProxy,void (WebKit::DrawingAreaProxy::*)(unsigned long long, const WebKit::UpdateInfo &),std::tuple<unsigned long long,WebKit::UpdateInfo>,0,1>(WebKit::DrawingAreaProxy * object, void(WebKit::DrawingAreaProxy::*)(unsigned __int64, const WebKit::UpdateInfo &) function, std::tuple<unsigned long long,WebKit::UpdateInfo> && args, std::integer_sequence<unsigned long long,0,1>) Line 42 C++ > WebKit2.dll!IPC::callMemberFunction<WebKit::DrawingAreaProxy,void (WebKit::DrawingAreaProxy::*)(unsigned long long, const WebKit::UpdateInfo &),std::tuple<unsigned long long,WebKit::UpdateInfo>,std::integer_sequence<unsigned long long,0,1> >(std::tuple<unsigned long long,WebKit::UpdateInfo> && args, WebKit::DrawingAreaProxy * object, void(WebKit::DrawingAreaProxy::*)(unsigned __int64, const WebKit::UpdateInfo &) function) Line 47 C++ > WebKit2.dll!IPC::handleMessage<Messages::DrawingAreaProxy::Update,WebKit::DrawingAreaProxy,void (WebKit::DrawingAreaProxy::*)(unsigned long long, const WebKit::UpdateInfo &)>(IPC::Decoder & decoder, WebKit::DrawingAreaProxy * object, void(WebKit::DrawingAreaProxy::*)(unsigned __int64, const WebKit::UpdateInfo &) function) Line 121 C++ > WebKit2.dll!WebKit::DrawingAreaProxy::didReceiveMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 44 C++ > WebKit2.dll!IPC::MessageReceiverMap::dispatchMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 124 C++ > WebKit2.dll!WebKit::AuxiliaryProcessProxy::dispatchMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 155 C++ > WebKit2.dll!WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection & connection, IPC::Decoder & decoder) Line 662 C++ > WebKit2.dll!IPC::Connection::dispatchMessage(IPC::Decoder & decoder) Line 940 C++ > WebKit2.dll!IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder,std::default_delete<IPC::Decoder> > message) Line 993 C++ > WebKit2.dll!IPC::Connection::dispatchIncomingMessages() Line 1097 C++ > WebKit2.dll!IPC::Connection::enqueueIncomingMessage::<unnamed-tag>::operator()() Line 914 C++ > WebKit2.dll!WTF::Detail::CallableWrapper<`lambda at ..\..\Source\WebKit\Platform\IPC\Connection.cpp:912:30',void>::call() Line 52 C++ > WTF.dll!WTF::Function<void ()>::operator()() Line 79 C++ > WTF.dll!WTF::RunLoop::performWork() Line 108 C++ > WTF.dll!WTF::RunLoop::wndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 57 C++ > WTF.dll!WTF::RunLoop::RunLoopWndProc(HWND__ * hWnd, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 39 C++ > [External Code] > WebKit.dll!WebKitMessageLoop::run(HACCEL__ * hAccelTable) Line 94 C++ > MiniBrowserLib.dll!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 97 C++ > MiniBrowserLib.dll!dllLauncherEntryPoint(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 115 C++ > MiniBrowser.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpstrCmdLine, int nCmdShow) Line 232 C++ > [External Code]
Fujii Hironori
Comment 9 2019-09-01 21:20:38 PDT
Created attachment 377836 [details] Wrong Patch
Fujii Hironori
Comment 10 2019-09-01 22:22:06 PDT
(In reply to Fujii Hironori from comment #6) > device scale factor is not correct in WinCairo WK2 Bug 196339 for this. > WinCairo WK2 crashes in some web sites Bug 196340 for this.
Brent Fulgham
Comment 11 2019-09-02 17:56:55 PDT
(In reply to Fujii Hironori from comment #10) > (In reply to Fujii Hironori from comment #6) > > device scale factor is not correct in WinCairo WK2 > > Bug 196339 for this. > > > WinCairo WK2 crashes in some web sites > > Bug 196340 for this. Hi Fujii -- do you intend to re-land this change when you have resolved the regressions you found? Or do you think this patch will not be necessary after that work is done?
Fujii Hironori
Comment 12 2019-09-02 18:42:12 PDT
I'll re-land this patch after finishing Bug 196339.
Brent Fulgham
Comment 13 2019-09-02 18:57:14 PDT
(In reply to Fujii Hironori from comment #12) > I'll re-land this patch after finishing Bug 196339. Excellent, thank you!
Note You need to log in before you can comment on or make changes to this bug.