Some sites do not display at HiDPI when using WebKit2GTK+. An example is https://duckduckgo.com/ It looks blurry on my HiDPI screen whereas https://google.com/ is fine. The version of WebKit2GTK+ is 2.6.5 https://www.archlinux.org/packages/extra/x86_64/webkit2gtk/ It can be reproduced using GNOME Web (Epiphany) or my own test web-browser: https://github.com/cjbayliss/web-browser
Two more example sites that are blurry are youtube.com and plus.google.com
HiDPI depends on the GTK+ and cairo versions found when building. What version of GTK+ and cairo are you using?
I'm now using Arch linux with GTK+ 3.16.1, Cario 1.14.2, and WebKit2GTK+ 2.8.0. I still have the same problem where some site are not HiDPI. I had this problem on Fedora 21 too. (GTK+ 3.14.x, WebKit2GTK+ 2.6.x, and I don't know the Cario version.) refs: https://www.archlinux.org/packages/extra/x86_64/cairo/ https://www.archlinux.org/packages/extra/x86_64/gtk3/
The 2.6.0 release notes say we have "HighDPI support for non-accelerated compositing contents" -- could that be the issue?
Which version of Cairo is required for proper HiDPI support?
I've noticed that some google sites (esp. the music player) initially start in HiDpi, and then drop down to "blurry" mode as the rest of the page starts to load. On the other hand, arstechnica.com seems to stay in hidpi mode.
It could be that HiDPI is not implemented in accelerated composited mode, so when switching to AC mode, we lose the HiDPI support.
Github is another example I've noticed that can be initially crisp then degrading while still loading. I'm finding that with v2.8.3 the page load starts crisp, then presumably when switching to accelerated compositing the content area flashes with some random corruption, then repaints itself blurry. (In reply to comment #5) > Which version of Cairo is required for proper HiDPI support? Cairo 1.14 introduced support for device scaling.
The reason for this problem is laking support of HiDPI from TextureMapper and TiledBackingStore. It renders contents in to unscaled surface(x 1) and composite it via scaled from (x 2) so the contents looks blurry. Now I'm trying to fix this because my new laptop has a HiDPI screen.
Yoon, how did this development turn out? You sounded like you almost had a fix last month, but I guess ran into troubles?
Created attachment 261640 [details] Patch
(In reply to comment #10) > Yoon, how did this development turn out? You sounded like you almost had a > fix last month, but I guess ran into troubles? After devoting amount of time, one problem left. You can see ugly part in my patch at DrawingAreaImpl.cpp
Awesome, thanks Yoon!
Created attachment 261733 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 261733 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=261733&action=review Thank you very much yoon. Patch works perfectly for me, I have only a few general questions/comments, since I'm not graphics expert. > Source/WebCore/ChangeLog:11 > + to apply transforms currectly. currectly -> correctly. > Source/WebCore/ChangeLog:14 > + TextureMapperBackingStore, and GraphicsLayerTextureMapper do not handles do not handles - does not handle > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:35 > + : m_contentsScale(1) > + , m_isScaleDirty(false) > + , m_isImage(false) Move these initializations to the header > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:93 > + if (!m_isImage) Could we just check if (m_image) ? and then we don't need the m_isImage member? > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:173 > + m_isImage = !!m_image; I don't get why we need m_isImage now.
Comment on attachment 261733 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=261733&action=review Looks good to me, minus a few minor nits. > Source/WebCore/platform/graphics/texmap/BitmapTexture.h:72 > + virtual void updateContents(TextureMapper*, GraphicsLayer*, const IntRect& target, const IntPoint& offset, UpdateContentsFlag, float scale = 1.0f); I think this can just be "scale = 1" here. > Source/WebCore/platform/graphics/texmap/TextureMapperTile.h:43 > + void updateContents(TextureMapper*, GraphicsLayer*, const IntRect&, BitmapTexture::UpdateContentsFlag UpdateCanModifyOriginalImageData, float scale = 1.0f); Ditto.
Created attachment 262143 [details] Patch
(In reply to comment #16) > Comment on attachment 261733 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=261733&action=review > > Thank you very much yoon. Patch works perfectly for me, I have only a few > general questions/comments, since I'm not graphics expert. > Nice! > > Source/WebCore/ChangeLog:11 > > + to apply transforms currectly. > > currectly -> correctly. > Thanks, I should enable a spell checker for my changelog editor. > > Source/WebCore/ChangeLog:14 > > + TextureMapperBackingStore, and GraphicsLayerTextureMapper do not handles > > do not handles - does not handle > Fixed. > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:35 > > + : m_contentsScale(1) > > + , m_isScaleDirty(false) > > + , m_isImage(false) > > Move these initializations to the header > Done. > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:93 > > + if (!m_isImage) > > Could we just check if (m_image) ? and then we don't need the m_isImage > member? > > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:173 > > + m_isImage = !!m_image; > > I don't get why we need m_isImage now. Nice catch. It was added to support seperated path for image uploading / and tile creations from my previous working set. It is not needed for this patch anywhere. It is Removed.
(In reply to comment #17) > Comment on attachment 261733 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=261733&action=review > > Looks good to me, minus a few minor nits. > > > Source/WebCore/platform/graphics/texmap/BitmapTexture.h:72 > > + virtual void updateContents(TextureMapper*, GraphicsLayer*, const IntRect& target, const IntPoint& offset, UpdateContentsFlag, float scale = 1.0f); > > I think this can just be "scale = 1" here. > > > Source/WebCore/platform/graphics/texmap/TextureMapperTile.h:43 > > + void updateContents(TextureMapper*, GraphicsLayer*, const IntRect&, BitmapTexture::UpdateContentsFlag UpdateCanModifyOriginalImageData, float scale = 1.0f); > > Ditto. Thanks, these are updated :)
Comment on attachment 262143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262143&action=review > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:166 > +void TextureMapperTiledBackingStore::setContentsToImage(Image* image) > +{ > + m_image = image; > } We can leave this in the header, as it currently is. > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.h:54 > + TextureMapperTiledBackingStore() > + : m_contentsScale(1), m_isScaleDirty(false) > + { } This is not what I meant, see below. > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.h:69 > + float m_contentsScale; > + bool m_isScaleDirty; float m_contentsScale { 1 }; bool m_isScaleDirty { false };
Created attachment 262147 [details] Patch
(In reply to comment #21) > Comment on attachment 262143 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=262143&action=review > > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.cpp:166 > > +void TextureMapperTiledBackingStore::setContentsToImage(Image* image) > > +{ > > + m_image = image; > > } > > We can leave this in the header, as it currently is. > Done. > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.h:54 > > + TextureMapperTiledBackingStore() > > + : m_contentsScale(1), m_isScaleDirty(false) > > + { } > > This is not what I meant, see below. > > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.h:69 > > + float m_contentsScale; > > + bool m_isScaleDirty; > > float m_contentsScale { 1 }; > bool m_isScaleDirty { false }; Ah, it looks like I'm far behind. Modified.
Comment on attachment 262147 [details] Patch Clearing flags on attachment: 262147 Committed r190344: <http://trac.webkit.org/changeset/190344>
(In reply to comment #21) > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.h:69 > > + float m_contentsScale; > > + bool m_isScaleDirty; > > float m_contentsScale { 1 }; > bool m_isScaleDirty { false }; You prefer this to float m_contentsScale = 1; bool m_isScaleDirty = false; ? I know the C++ folks are saying to use bracket initialization everywhere to prevent narrowing, but we don't do that anywhere in WebKit and the style checker usually complains (I guess not in a header file, though).
(In reply to comment #25) > (In reply to comment #21) > > > Source/WebCore/platform/graphics/texmap/TextureMapperTiledBackingStore.h:69 > > > + float m_contentsScale; > > > + bool m_isScaleDirty; > > > > float m_contentsScale { 1 }; > > bool m_isScaleDirty { false }; > > You prefer this to > > float m_contentsScale = 1; > bool m_isScaleDirty = false; > > ? I know the C++ folks are saying to use bracket initialization everywhere > to prevent narrowing, but we don't do that anywhere in WebKit and the style > checker usually complains (I guess not in a header file, though). We use brackets everywhere in newly written WebKit code