[Webkit-unassigned] [Bug 110193] OpenCL implementation of FEMerge filter.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 19 02:46:53 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=110193
Zoltan Herczeg <zherczeg at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #189025|review?, commit-queue? |review-, commit-queue-
Flag| |
--- Comment #2 from Zoltan Herczeg <zherczeg at webkit.org> 2013-02-19 02:49:15 PST ---
(From update of attachment 189025)
Nice work, mostly style comments:
View in context: https://bugs.webkit.org/attachment.cgi?id=189025&action=review
> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.h:59
> + , m_copy(0)
m_mergeCopyOperation
> Source/WebCore/platform/graphics/gpu/opencl/FilterContextOpenCL.h:99
> + inline void copy(OpenCLHandle, IntSize, OpenCLHandle, IntRect);
applyFEMergeCopy
> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEMerge.cpp:42
> + float4 destinationPixel = sourcePixel + destinationPixel * (1 - sourcePixel.w);
What is the purpose of this line? Especially as this is a copy operation? The sourcePixel should be renamed to destinationPixel, and no need for any modifications.
> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEMerge.cpp:48
> + int2 coord = (int2) (get_global_id(0), get_global_id(1));
destinationCoord
> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEMerge.cpp:57
> +
> + destinationPixel = sourcePixelA + destinationPixel * (1 - sourcePixelA.w);
> +
> + destinationPixel = sourcePixelB + destinationPixel * (1 - sourcePixelB.w);
We don't need that many empty lines here.
> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEMerge.cpp:59
> + write_imagef(previousDestination, coord, destinationPixel);
??? You write data to the previous destination? The destination should be the target of the operation, and previousDestination should be the source. __write_only image2d_t destination
> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEMerge.cpp:138
> + OpenCLHandle previousDestination = context->createOpenCLImage(absolutePaintRect().size());
> + context->fill(previousDestination, absolutePaintRect().size(), Color(0.0f, 0.0f, 0.0f, 0.0f));
OpenCLHandle previousDestination = destination;
destination = context->createOpenCLImage(absolutePaintRect().size());
> Source/WebCore/platform/graphics/gpu/opencl/OpenCLFEMerge.cpp:153
> + context->applyFEMerge(destination, previousDestination, absolutePaintRect().size(), sourceA, relativeSourceRectA, sourceB, relativeSourceRectB);
This does not match to __kernel void merge. The applyFEMerge should have to same order of arguments.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list