[webkit-reviews] review denied: [Bug 100026] inconsistency in drawImage with target rect negative dimensions. : [Attachment 171309] fix calls in FEComposite and in Gstreamer; replace IntPoint(0, 0) with IntPoint()

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied arno. <arno at renevier.net>'s request
for review:
Bug 100026: inconsistency in drawImage with target rect negative dimensions.
https://bugs.webkit.org/show_bug.cgi?id=100026

Attachment 171309: fix calls in FEComposite and in Gstreamer; replace
IntPoint(0, 0) with IntPoint()
https://bugs.webkit.org/attachment.cgi?id=171309&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=171309&action=review


> Source/WebCore/platform/graphics/GraphicsContext.cpp:443
> +    if (!image)
> +	   return;

I would really like ti have all these functions have ASSERT(!image) instead.
Can you check if it is possible to check for the image before calling this
functions?

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1626

> -    context->drawImage(reinterpret_cast<Image*>(gstImage->image().get()),
ColorSpaceSRGB,
> -			  rect, gstImage->rect(), CompositeCopy,
DoNotRespectImageOrientation, false);
> +    Image* image = reinterpret_cast<Image*>(gstImage->image().get());
> +    if (!image)
> +	   return;
> +
> +    FloatRect srcRect = gstImage->cropRect();
> +    if (srcRect.isEmpty())
> +	   srcRect  = FloatRect(IntRect(IntPoint(), image->size()));

I assume that gstImage->rect() implies that -1, -1 means undetermined intrinsic
lines. Does the GStreamer code make the same assumption somewhere else? Why did
you change to gstImage->cropRect()? Can we may fix gstImage->rect() instead?