This is only a problem when software rendering is used. The compositor already handles this properly.
Created attachment 147402 [details] Patch Possible fix. Not sure if this is how we want solve this.
https://bugs.webkit.org/show_bug.cgi?id=89031 happens to solve this.
Comment on attachment 147402 [details] Patch I think you'd be better off calling setDrawingToImageBuffer() in NonCompositedContentHost, or just waiting for https://bugs.webkit.org/show_bug.cgi?id=89031
(In reply to comment #2) > https://bugs.webkit.org/show_bug.cgi?id=89031 happens to solve this. (In reply to comment #3) > (From update of attachment 147402 [details]) > I think you'd be better off calling setDrawingToImageBuffer() in NonCompositedContentHost, or just waiting for https://bugs.webkit.org/show_bug.cgi?id=89031 I'm confused. This patch is to avoid sub-pixel rendering in software mode. Not when the compositor is used which is already handled correctly. So changes to NonCompositedContentHost isn't going to help here afaik. Tried the patch attached to https://bugs.webkit.org/show_bug.cgi?id=89031 and it doesn't fix the problem.
Comment on attachment 147402 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147402&action=review > Source/WebKit/chromium/src/WebPagePopupImpl.cpp:240 > + PageWidgetDelegate::paint(m_page.get(), 0, canvas, rect, !m_webView->isTransparent()); this isn't obvious to me - if a web view is transparent, does that really mean that popups associated with that view are also transparent? how is the background color of a popup determined?
Ah sorry, I didn't realize this was for software mode.
(In reply to comment #5) > (From update of attachment 147402 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147402&action=review > > > Source/WebKit/chromium/src/WebPagePopupImpl.cpp:240 > > + PageWidgetDelegate::paint(m_page.get(), 0, canvas, rect, !m_webView->isTransparent()); > > this isn't obvious to me - if a web view is transparent, does that really mean that popups associated with that view are also transparent? how is the background color of a popup determined? yea, that's wrong. popups are always opaque afaik. WebPagePopupImpl::initPage() calls frame->view()->setTransparent(false). btw, not sure how to test this other than the manual testing I'm doing on chromeos right now. any ideas?
Created attachment 147626 [details] Patch Popups are always opaque
(In reply to comment #7) > (In reply to comment #5) > > (From update of attachment 147402 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=147402&action=review > > > > > Source/WebKit/chromium/src/WebPagePopupImpl.cpp:240 > > > + PageWidgetDelegate::paint(m_page.get(), 0, canvas, rect, !m_webView->isTransparent()); > > > > this isn't obvious to me - if a web view is transparent, does that really mean that popups associated with that view are also transparent? how is the background color of a popup determined? > > yea, that's wrong. popups are always opaque afaik. WebPagePopupImpl::initPage() calls frame->view()->setTransparent(false). > > btw, not sure how to test this other than the manual testing I'm doing on chromeos right now. any ideas? So sadly we force grayscale all the time with layout tests, so there's no way that I know of in WebKit to check this automatically :(
Comment on attachment 147626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147626&action=review R=me but consider 2-state enum instead of bool for the paint parameter > Source/WebKit/chromium/src/WebPagePopupImpl.cpp:240 > + PageWidgetDelegate::paint(m_page.get(), 0, canvas, rect, true); this is a good sign that we want a two-state enum here instead of a bool - it's really hard to guess what "true" means in this context.
Created attachment 147650 [details] Patch Add two-state enum
Comment on attachment 147650 [details] Patch Clearing flags on attachment: 147650 Committed r120381: <http://trac.webkit.org/changeset/120381>
All reviewed patches have been landed. Closing bug.