Re: RFR: 8242861: Update ImagePattern to apply SVG pattern transforms [v7]
On Mon, 12 Oct 2020 03:11:18 GMT, Arun Joseph wrote: >> fillPath() and fillRect() functions in >> [GraphicsContextJava.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/GraphicsContextJava.cpp) >> use Image::drawPattern() for applying patterns as fill. But drawPattern() >> doesn't use patternTransform argument as ImagePattern doesn't have the same >> attribute. So, the final image won't be transformed. > > Arun Joseph has updated the pull request incrementally with one additional > commit since the last revision: > > Fix incorrect concat param order Marked as reviewed by arapte (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/190
Re: RFR: 8242861: Update ImagePattern to apply SVG pattern transforms [v7]
On Wed, 4 Nov 2020 15:41:12 GMT, Arun Joseph wrote:
>> modules/javafx.graphics/src/main/java/com/sun/prism/impl/ps/PaintHelper.java
>> line 754:
>>
>>> 752:
>>> 753: BaseTransform paintXform = paint.getPatternTransformNoClone();
>>> 754: if (paintXform != null) {
>>
>> Minor: Is the null check needed ? I could not find an instance where an
>> object of `ImagePattern` is constructed using default constructor.
>> If `ImagePattern.getPatternTransformNoClone()` could return null then should
>> we do null check with other calls to
>> `ImagePattern.getPatternTransformNoClone()` ? OR may be remove this null
>> check.
>
> The null check is not actually required as instances of `ImagePattern` can't
> have a null `patternTransform`. I added this check as `setLinearGradient` and
> `setRadialGradient` functions have also added them, when instances of
> `Gradient` can't have null as its transform. Should this null check be
> removed?
I think leaving it for consistency is fine.
-
PR: https://git.openjdk.java.net/jfx/pull/190
Re: RFR: 8242861: Update ImagePattern to apply SVG pattern transforms [v7]
On Tue, 3 Nov 2020 12:01:38 GMT, Ambarish Rapte wrote:
>> Arun Joseph has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix incorrect concat param order
>
> modules/javafx.graphics/src/main/java/com/sun/prism/impl/ps/PaintHelper.java
> line 754:
>
>> 752:
>> 753: BaseTransform paintXform = paint.getPatternTransformNoClone();
>> 754: if (paintXform != null) {
>
> Minor: Is the null check needed ? I could not find an instance where an
> object of `ImagePattern` is constructed using default constructor.
> If `ImagePattern.getPatternTransformNoClone()` could return null then should
> we do null check with other calls to
> `ImagePattern.getPatternTransformNoClone()` ? OR may be remove this null
> check.
The null check is not actually required as instances of `ImagePattern` can't
have a null `patternTransform`. I added this check as `setLinearGradient` and
`setRadialGradient` functions have also added them, when instances of
`Gradient` can't have null as its transform. Should this null check be removed?
-
PR: https://git.openjdk.java.net/jfx/pull/190
Re: RFR: 8242861: Update ImagePattern to apply SVG pattern transforms [v7]
On Mon, 12 Oct 2020 03:11:18 GMT, Arun Joseph wrote:
>> fillPath() and fillRect() functions in
>> [GraphicsContextJava.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/GraphicsContextJava.cpp)
>> use Image::drawPattern() for applying patterns as fill. But drawPattern()
>> doesn't use patternTransform argument as ImagePattern doesn't have the same
>> attribute. So, the final image won't be transformed.
>
> Arun Joseph has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fix incorrect concat param order
Added a small query comment.
Rest looks good to me, change seems specific to `ImagePattern` transformation.
This transformation is set only when specified from webkit and is identity
otherwise.
modules/javafx.graphics/src/main/java/com/sun/prism/impl/ps/PaintHelper.java
line 754:
> 752:
> 753: BaseTransform paintXform = paint.getPatternTransformNoClone();
> 754: if (paintXform != null) {
Minor: Is the null check needed ? I could not find an instance where an object
of `ImagePattern` is constructed using default constructor.
If `ImagePattern.getPatternTransformNoClone()` could return null then should we
do null check with other calls to `ImagePattern.getPatternTransformNoClone()` ?
OR may be remove this null check.
-
PR: https://git.openjdk.java.net/jfx/pull/190
Re: RFR: 8242861: Update ImagePattern to apply SVG pattern transforms [v7]
On Mon, 12 Oct 2020 03:11:18 GMT, Arun Joseph wrote: >> fillPath() and fillRect() functions in >> [GraphicsContextJava.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/GraphicsContextJava.cpp) >> use Image::drawPattern() for applying patterns as fill. But drawPattern() >> doesn't use patternTransform argument as >> ImagePattern doesn't have the same attribute. So, the final image won't be >> transformed. > > Arun Joseph has updated the pull request incrementally with one additional > commit since the last revision: > > Fix incorrect concat param order Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.java.net/jfx/pull/190
Re: RFR: 8242861: Update ImagePattern to apply SVG pattern transforms [v7]
> fillPath() and fillRect() functions in > [GraphicsContextJava.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/GraphicsContextJava.cpp) > use Image::drawPattern() for applying patterns as fill. But drawPattern() > doesn't use patternTransform argument as > ImagePattern doesn't have the same attribute. So, the final image won't be > transformed. Arun Joseph has updated the pull request incrementally with one additional commit since the last revision: Fix incorrect concat param order - Changes: - all: https://git.openjdk.java.net/jfx/pull/190/files - new: https://git.openjdk.java.net/jfx/pull/190/files/4241c502..abb4787a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=190=06 - incr: https://webrevs.openjdk.java.net/?repo=jfx=190=05-06 Stats: 4 lines in 2 files changed: 1 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/190.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/190/head:pull/190 PR: https://git.openjdk.java.net/jfx/pull/190
