Bug 147589

Summary: feMorphology is not rendered correctly on Retina display
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: ImagesAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, kondapallykalyan, rcombs, zimmermann
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
test case
none
Patch
none
Patch none

Said Abou-Hallawa
Reported 2015-08-03 14:06:49 PDT
Created attachment 258111 [details] test case Open the attached test case which contains an text SVG element with feMorphology filter on Retina display. Result: The display is corrupted. Note: If the same test case is opened on non-Retina display, the text is displayed fine.
Attachments
test case (420 bytes, text/html)
2015-08-03 14:06 PDT, Said Abou-Hallawa 		no flags
Details
Patch (4.16 KB, patch)
2015-08-04 15:49 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Patch (4.17 KB, patch)
2015-08-04 18:30 PDT, Said Abou-Hallawa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Said Abou-Hallawa
Comment 1 2015-08-04 13:51:31 PDT
<rdar://problem/22098031>
Said Abou-Hallawa
Comment 2 2015-08-04 15:49:46 PDT
Created attachment 258224 [details] Patch
Said Abou-Hallawa
Comment 3 2015-08-04 18:30:25 PDT
Created attachment 258254 [details] Patch
WebKit Commit Bot
Comment 4 2015-08-11 12:27:33 PDT
Comment on attachment 258254 [details] Patch Clearing flags on attachment: 258254 Committed r188271: <http://trac.webkit.org/changeset/188271>
WebKit Commit Bot
Comment 5 2015-08-11 12:27:36 PDT
All reviewed patches have been landed. Closing bug.
alan
Comment 6 2015-08-12 19:11:10 PDT
Comment on attachment 258254 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=258254&action=review > Source/WebCore/platform/graphics/filters/FEMorphology.cpp:244 > + paintingData.width = ceilf(effectDrawingRect.width() * filter.filterScale()); > + paintingData.height = ceilf(effectDrawingRect.height() * filter.filterScale()); > + paintingData.radiusX = ceilf(radiusX * filter.filterScale()); > + paintingData.radiusY = ceilf(radiusY * filter.filterScale()); Why are we ceiling both the location and the dimension here?
Said Abou-Hallawa
Comment 7 2015-08-13 10:29:01 PDT
(In reply to comment #6) > Comment on attachment 258254 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=258254&action=review > > > Source/WebCore/platform/graphics/filters/FEMorphology.cpp:244 > > + paintingData.width = ceilf(effectDrawingRect.width() * filter.filterScale()); > > + paintingData.height = ceilf(effectDrawingRect.height() * filter.filterScale()); > > + paintingData.radiusX = ceilf(radiusX * filter.filterScale()); > > + paintingData.radiusY = ceilf(radiusY * filter.filterScale()); > > Why are we ceiling both the location and the dimension here? Is it wrong to do ceiling here? The filterScale() is set only to renderer().frame().page()->deviceScaleFactor() which is float. But it is 2 on Retina and 1 on none Retina. Can it be for example 1.5? I do not know. So I assumed filterScale() is just a float and can be any value. Now I have floating point calculations even the variables before scaling and after scaling are all integers. I thought it is safer to do ceiling to avoid not setting the bits of the last row or the last column if filterScale() has a fraction. And since we are dealing with a memory bitmap, we are sure that nothing will be drawn outside the image pre allocated area.
Note You need to log in before you can comment on or make changes to this bug.