Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v5]

2020-07-21 Thread Bhawesh Choudhary
On Fri, 12 Jun 2020 22:01:47 GMT, Kevin Rushforth  wrote:

>> The code looks good (with a couple minor formatting issues).
>> 
>> All of the onscreen testing I did  looks good on Windows. I'd like to test 
>> it on Mac as well.
>> 
>> There is an issue with printing in the case of Hi-DPI scaling, which is what 
>> I run by default on Windows. The gradient
>> texture appears to be scaled incorrectly (as if the scale was applied more 
>> than once). If I force scaling to 1 with
>> `-Dglass.win.uiScale=1` then it prints correctly.
>
> It behaves the same on Mac with a Retina display as it does on Windows with 
> Hi-DPI scaling. The gradient doesn't appear
> to be scaled correctly when printing. It's fine with on-screen rendering 
> (with both HW and SW pipeline).

Issue related to hidpi rendering was caused due to correct pixel scale factor 
not being set to the context in which
mask texture was getting rendered. setting correct device scale factor in 
`RenderSVGResourceMasker.cpp` fixed the
issue. Below images shows the rendered mask texture in both case (HiDpi and 
Normal respectively)

![HiDpi_Mask_1](https://user-images.githubusercontent.com/4208131/88110579-125ae300-cbca-11ea-9c02-ceec2ccdf7d1.png)
![Normal_Mask_1](https://user-images.githubusercontent.com/4208131/88110582-138c1000-cbca-11ea-88ce-5e15d124.png)

Another issue was in Hi DPI printing. PrintGraphics draws with different 
resolution than the mask texture. Before the
fix mask was not drawn correctly to RTTexture, due to which only top left 
portion which comes inside current draw
bounds was taken to draw the whole image. After fix, entire mask texture is 
always considered while doing final drawing.

-

PR: https://git.openjdk.java.net/jfx/pull/213


Re: RFR: 8218973: SVG with masking is not rendering image with mask effect [v6]

2020-07-21 Thread Bhawesh Choudhary
> Root cause of issue is Specifying a image mask from GraphicsContextJava.cpp 
> in WebKit was not implemented, so masking
> doesn't take place at all while rendering SVGRect. to fix this issue add 
> implementation of function clipToImageBuffer()
> in GraphicsContextJava.cpp and send clip image to WCGraphicsPrismContext.java 
>  While rendering in
> WCGraphicsPrismContext.java if image clip mask is available, use it for 
> rendering using MaskTextureGraphics interface
> otherwise use usual way of rendering.

Bhawesh Choudhary has updated the pull request incrementally with one 
additional commit since the last revision:

  HiDPI printing and Rendering fix

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/213/files
  - new: https://git.openjdk.java.net/jfx/pull/213/files/f0c217ec..312d068a

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/213/webrev.05
 - incr: https://webrevs.openjdk.java.net/jfx/213/webrev.04-05

  Stats: 34 lines in 2 files changed: 5 ins; 9 del; 20 mod
  Patch: https://git.openjdk.java.net/jfx/pull/213.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/213/head:pull/213

PR: https://git.openjdk.java.net/jfx/pull/213


Re: RFR: 8241840: Memoryleak: Closed focused Stages are not collected with Monocle. [v2]

2020-07-21 Thread Kevin Rushforth
On Tue, 21 Jul 2020 12:33:28 GMT, Florian Kirmaier  
wrote:

>> I don't think the class MeshManagerCacheLeakTest is a good base to write 
>> monocle + native tests.
>> It required input/output parsing which would be a bit too much.
>> 
>> In my latest commit, I've added a simple solution to how the test can be 
>> reused.
>> On my side, both tests are always green, but I'm using a mac.
>> 
>> 
>> If one of the tests is unstable on windows, then it would be great if we 
>> could consider it as 2 bugs, so this PR can be
>> finished.
>> Afterwards it would be great if someone else could continue the windows-bug 
>> because I don't have a very productive
>> setup to work on it and I also don't really understand the native windows 
>> code.
>
> Any updates about this PR?
> I run into this bug basically every time I check something for a memory leak, 
> which is quite annoying.
> Edit: The rerequest review button seems to be not working

Once the last of the `jfx15` reviews and other critical reviews are out of the 
way, we'll get back to this for JavaFX
16.

-

PR: https://git.openjdk.java.net/jfx/pull/153


Re: [jfx15] RFR: 8220484: JFXPanel renders a slanted image with a hidpi monitor scale of 125% or 175% [v6]

2020-07-21 Thread Kevin Rushforth
On Mon, 20 Jul 2020 18:11:34 GMT, Oliver Schmidtmer 
 wrote:

>> In edge cases where monitor scaling of 1.25 or 1.75 is active, Math.ceil and 
>> Math.round produce different results and
>> EmbeddedScene#getPixels in JFXPanel#paintComponent causes an off-by-one 
>> error on the line width and therefore sheared
>> rendering.  The changes were already proposed by the submitter in 
>> JBS-8220484.
>> 
>> OCA is signed and send.
>
> Oliver Schmidtmer has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - run test only on windows
>  - Typo

Looks good.

@prsadhuk can you also review this?

-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/246


Re: RFR: 8241840: Memoryleak: Closed focused Stages are not collected with Monocle. [v5]

2020-07-21 Thread Florian Kirmaier
On Fri, 17 Apr 2020 12:57:28 GMT, Ambarish Rapte  wrote:

>> The previous version of test failed on my Windows 10 machine with below 
>> exception, but the updated version does not
>> fail,
>> test.javafx.stage.FocusedWindowTest > testLeak FAILED
>> junit.framework.AssertionFailedError: Expected:  but was: 
>> javafx.stage.Stage@5fe60662
>> at junit.framework.Assert.fail(Assert.java:47)
>> at junit.framework.Assert.assertTrue(Assert.java:20)
>> at junit.framework.Assert.assertNull(Assert.java:233)
>> at junit.framework.Assert.assertNull(Assert.java:226)
>> at 
>> test.javafx.stage.FocusedWindowTest.assertCollectable(FocusedWindowTest.java:133)
>> at 
>> test.javafx.stage.FocusedWindowTest.testLeakOnce(FocusedWindowTest.java:116)
>> at 
>> test.javafx.stage.FocusedWindowTest.testLeak(FocusedWindowTest.java:84)
>
>> Do you have a good recommendation on how to add it?
> 
> May be a test like `MeshManagerCacheLeakTest` should be able to solve the 
> problem. I have not tried it myself, please
> check if it can help here.

See below command

-

PR: https://git.openjdk.java.net/jfx/pull/153


Re: RFR: 8241840: Memoryleak: Closed focused Stages are not collected with Monocle. [v2]

2020-07-21 Thread Florian Kirmaier
On Wed, 22 Apr 2020 12:00:03 GMT, Florian Kirmaier  
wrote:

>> Changes requested by arapte (Reviewer).
>
> I don't think the class MeshManagerCacheLeakTest is a good base to write 
> monocle + native tests.
> It required input/output parsing which would be a bit too much.
> 
> In my latest commit, I've added a simple solution to how the test can be 
> reused.
> On my side, both tests are always green, but I'm using a mac.
> 
> 
> If one of the tests is unstable on windows, then it would be great if we 
> could consider it as 2 bugs, so this PR can be
> finished.
> Afterwards it would be great if someone else could continue the windows-bug 
> because I don't have a very productive
> setup to work on it and I also don't really understand the native windows 
> code.

Any updates about this PR?
I run into this bug basically every time I check something for a memory leak, 
which is quite annoying.
Edit: The rerequest review button seems to be not working

-

PR: https://git.openjdk.java.net/jfx/pull/153


Re: RFR: 8244297: memory leak test utility [v4]

2020-07-21 Thread Florian Kirmaier
On Thu, 7 May 2020 18:16:22 GMT, Florian Kirmaier  wrote:

>> What I tried asking from my work account is: what is the purpose of 
>> createGarbage?
>
> The createGarbage method stimulates the GC. All unit tests are green without 
> it, but according to my memory some tests
> in an earlier version of this library were unstable without this. I can also 
> change the configuration to create 0
> garbage.

Any news about this PR?

-

PR: https://git.openjdk.java.net/jfx/pull/204


Re: RFR: 8241840: Memoryleak: Closed focused Stages are not collected with Monocle. [v2]

2020-07-21 Thread Florian Kirmaier
On Wed, 22 Apr 2020 12:00:03 GMT, Florian Kirmaier  
wrote:

>> Changes requested by arapte (Reviewer).
>
> I don't think the class MeshManagerCacheLeakTest is a good base to write 
> monocle + native tests.
> It required input/output parsing which would be a bit too much.
> 
> In my latest commit, I've added a simple solution to how the test can be 
> reused.
> On my side, both tests are always green, but I'm using a mac.
> 
> 
> If one of the tests is unstable on windows, then it would be great if we 
> could consider it as 2 bugs, so this PR can be
> finished.
> Afterwards it would be great if someone else could continue the windows-bug 
> because I don't have a very productive
> setup to work on it and I also don't really understand the native windows 
> code.

Any news about this PR?
It's a bit annoying to explain everyone - who is checking for memory leaks - 
this bug.

-

PR: https://git.openjdk.java.net/jfx/pull/153


[jfx15] Integrated: 8248381: Create a daemon thread for MonocleTimer

2020-07-21 Thread John Neffenger
On Fri, 26 Jun 2020 03:50:02 GMT, John Neffenger 
 wrote:

> Fixes [JDK-8248381](https://bugs.openjdk.java.net/browse/JDK-8248381).

This pull request has now been integrated.

Changeset: 5f60ea5d
Author:John Neffenger 
Committer: Kevin Rushforth 
URL:   https://git.openjdk.java.net/jfx/commit/5f60ea5d
Stats: 7 lines in 1 file changed: 0 ins; 6 del; 1 mod

8248381: Create a daemon thread for MonocleTimer

Reviewed-by: kcr, jvos

-

PR: https://git.openjdk.java.net/jfx/pull/256