r105515 broke reflections when using text zoom to zoom out to a level < 1. The reflection mask is truncated.
Created attachment 144612 [details] Testcase
<rdar://problem/11387506>
Niko: ping
It seems like RenderBoxModelObject::calculateImageIntrinsicDimensions() does not always scale by the effectiveZoom so it doesn't make sense to always divide the zoom out. That's what happening in this test case. That is what is happening for the attached reduction. We return right away without scaling the dimensions: IntSize RenderBoxModelObject::calculateImageIntrinsicDimensions(StyleImage* image, const IntSize& positioningAreaSize) const { // A generated image without a fixed size, will always return the container size as intrinsic size. if (image->isGeneratedImage() && image->usesImageContainerSize()) return IntSize(positioningAreaSize.width(), positioningAreaSize.height()); … But there are other cases that don't scale too.
Created attachment 145361 [details] Patch Here's one way to fix this.
Comment on attachment 145361 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145361&action=review > Source/WebCore/ChangeLog:17 > + * rendering/RenderBoxModelObject.cpp: Additional newline here. > Source/WebCore/rendering/RenderBoxModelObject.cpp:1034 > + IntSize imageIntrinsicSize = calculateImageIntrinsicDimensions(image, positioningAreaSize, true); Maybe an enum instead of an incomprehensible bool argument? > Source/WebCore/rendering/RenderBoxModelObject.cpp:1251 > - int imageWidth = imageSize.width() / style->effectiveZoom(); > - int imageHeight = imageSize.height() / style->effectiveZoom(); > + int imageWidth = imageSize.width(); > + int imageHeight = imageSize.height(); Have you quadruple checked that this doesn't break the apple.com top bar again (a la https://bugs.webkit.org/show_bug.cgi?id=76249)? > LayoutTests/ChangeLog:5 > + -and corresponding- I don't know that the -and corresponding- is necessary, but OK :D
Comment on attachment 145361 [details] Patch r+ but I agree with Tim!
(In reply to comment #6) > (From update of attachment 145361 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=145361&action=review > > > Source/WebCore/ChangeLog:17 > > + * rendering/RenderBoxModelObject.cpp: > > Additional newline here. > Added. > > Source/WebCore/rendering/RenderBoxModelObject.cpp:1034 > > + IntSize imageIntrinsicSize = calculateImageIntrinsicDimensions(image, positioningAreaSize, true); > > Maybe an enum instead of an incomprehensible bool argument? > I knew someone was going to suggest this. And I only didn't do it that way from the start because I suck so much at naming things. How about: enum ShouldScaleByEffectiveZoom { ShouldScale, ShouldNotScale } Thoughts? Suggestions? > > Source/WebCore/rendering/RenderBoxModelObject.cpp:1251 > > - int imageWidth = imageSize.width() / style->effectiveZoom(); > > - int imageHeight = imageSize.height() / style->effectiveZoom(); > > + int imageWidth = imageSize.width(); > > + int imageHeight = imageSize.height(); > > Have you quadruple checked that this doesn't break the apple.com top bar again (a la https://bugs.webkit.org/show_bug.cgi?id=76249)? > I have! I ran lots of layout tests and pixel tests too. > > LayoutTests/ChangeLog:5 > > + -and corresponding- > > I don't know that the -and corresponding- is necessary, but OK :D It's my thang! Just ask mitzpettel. :-P
Fixed: http://trac.webkit.org/changeset/119274
(In reply to comment #3) > Niko: ping Sorry for the late answer - I was busy with internal work this week. Beth, thanks a bunch for looking into this!
(In reply to comment #10) > (In reply to comment #3) > > Niko: ping > Sorry for the late answer - I was busy with internal work this week. > Beth, thanks a bunch for looking into this! No worries, I understand. And, you're welcome! :-)