Review request for JDK-8170140: rendering anomaly with MarlinFX renderer

2016-11-23 Thread Laurent Bourgès
Jim & Kevin,

Please review the marlin-FX webrev:
http://cr.openjdk.java.net/~lbourges/marlinFX/marlinFX-8170140.0/
JBS: https://bugs.openjdk.java.net/browse/JDK-8170140

Ths bug fix seems very simple in MaskMarlinAlphaConsumer:

+// ensure last block flag = 0 to process final block:+
blkFlags[blkE] = 0;

But it is less easy to explain:
In Renderer, the block flags are set by:
blkFlags[(pix_xmax + 1)  >> _BLK_SIZE_LG] = 1

I recently added +1 that can lead to flag an extra block when pix_xmax is
at a block boundary (32 pixels).

pix_xmax = x1 >> _SUBPIXEL_LG_POSITIONS_X
x1 <= bboxx1 = bbox_spmaxX = pmaxX << SUBPIXEL_LG_POSITIONS_X

So pix_xmax is at most equals to pmaxX = bboxX1 = getOutpixMaxX()

The added line in MaskMarlinAlphaConsumer ensures that the
setAndClearRelativeAlphas() always process the loop that clears and copies
the last block of pixels:
blkFlags[blkE] = 0 is equivalent to blkFlags[(bboxX1 + 1)  >> _BLK_SIZE_LG]
= 0

This is quite difficult to explain but it is now correct.

I added a test in System tests as it was not working in javafx.graphics
tests (Exception in Application constructor)

PS: This bug may be the cause of RegionBackgroundFillUITest failures:
https://bugs.openjdk.java.net/browse/JDK-8170026

Regards,
Laurent


Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Semyon Sadetsky

On 23.11.2016 19:29, Tom Schindl wrote:


Hi,

Ok. I think I got it:

a) if SWT is loaded first (which is th 99% case I guess) then you are
finding out that you need to run with gtk2 or gtk3 by finding out
what gtk libs are already loaded (webref
http://cr.openjdk.java.net/~ssadetsky/8156491/webrev.00/)

b) if JavaFX is loaded before SWT but SWT is on the classpath then you
try to findout what SWT will load and align with that (webref
http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.01/)

If that is correct all my concerns are addressed! There's one very
unlikely case:

a) one creates an OSGi-Application
It is possible that SWT lib is added to an another OSGi bundle classpath 
which is not accessible. Since all OSGi bundles share the same process 
it may cause the GTK version conflict. I think we cannot resolve this 
issue for OSGi and for all other custom ClassLoader hierarchies.


--Semyon

b) one loads a JavaFX UI
c) loads an SWT-UI

Both webrefs won't address this case if not mistaken, but I have never
seen an application like that ;-)

Tom

On 23.11.16 13:32, Semyon Sadetsky wrote:

On 11/23/2016 12:38 PM, Tom Schindl wrote:


  From looking at the code I doubt your fix will work reliably in a
OSGi-Environment who is the Main deployment scenario for SWT and hence
FXCanvas!

For sure you won't find the SWT-Library on the SystemClassloader and
whether you find it on the Thread-ContextClassloader is just gambling!

The only area you for sure can detect the swt-library are the
swt-fx.jar-Classes because those are guaranteed to be loaded with a
classloader who can look up SWT-Libary classes like
"org.eclipse.swt.internal.gtk.OS"

What is swt-fx.jar? Maybe you meant javafx-swt.jar?

BTW: Is the statement below really correct?

 in JFX embeded into SWT scenarios JFX loads GTK primarily ...

If you create an instance of FXCanvas SWT must have been loaded already
(You need to pass a parent Composite) so the native gtk-libs are there
already loaded.

That's right concern. Actually the fix covers the scenario when swt.jar
is on the classpath and may be potentially loaded in future with another
GTK version (which will cause the process crash). The rest scenarios are
covered by the 8156491 fix which I just posted for review.

--Semyon

Tom

On 22.11.16 14:51, Semyon Sadetsky wrote:

Hello Kevin & David,

Please review the fix for jfx9:

bug: https://bugs.openjdk.java.net/browse/JDK-8157002

webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/

In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
version cannot be detected using the check for the loaded native library
version.

The fix proposes a way to detect if GTK version of swt.jar library is
available on the classpath and tries to get the GTK version from SWT lib
internal java classes.

--Semyon







Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Kevin Rushforth

Thanks, Tom.

-- Kevin


Tom Schindl wrote:

Hi Kevin,

Just replied to Semyon and I think the change is fine

Tom

On 23.11.16 16:37, Kevin Rushforth wrote:
  

Hi Tom,

Would you have time to help us test this?

-- Kevin


Tom Schindl wrote:


From looking at the code I doubt your fix will work reliably in a
OSGi-Environment who is the Main deployment scenario for SWT and hence
FXCanvas!

For sure you won't find the SWT-Library on the SystemClassloader and
whether you find it on the Thread-ContextClassloader is just gambling!

The only area you for sure can detect the swt-library are the
swt-fx.jar-Classes because those are guaranteed to be loaded with a
classloader who can look up SWT-Libary classes like
"org.eclipse.swt.internal.gtk.OS"

BTW: Is the statement below really correct?

 in JFX embeded into SWT scenarios JFX loads GTK primarily ...

If you create an instance of FXCanvas SWT must have been loaded already
(You need to pass a parent Composite) so the native gtk-libs are there
already loaded.

Tom

On 22.11.16 14:51, Semyon Sadetsky wrote:
  
  

Hello Kevin & David,

Please review the fix for jfx9:

bug: https://bugs.openjdk.java.net/browse/JDK-8157002

webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/

In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
version cannot be detected using the check for the loaded native library
version.

The fix proposes a way to detect if GTK version of swt.jar library is
available on the classpath and tries to get the GTK version from SWT lib
internal java classes.

--Semyon



  
  



  


Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Tom Schindl
Hi Kevin,

Just replied to Semyon and I think the change is fine

Tom

On 23.11.16 16:37, Kevin Rushforth wrote:
> Hi Tom,
> 
> Would you have time to help us test this?
> 
> -- Kevin
> 
> 
> Tom Schindl wrote:
>> From looking at the code I doubt your fix will work reliably in a
>> OSGi-Environment who is the Main deployment scenario for SWT and hence
>> FXCanvas!
>>
>> For sure you won't find the SWT-Library on the SystemClassloader and
>> whether you find it on the Thread-ContextClassloader is just gambling!
>>
>> The only area you for sure can detect the swt-library are the
>> swt-fx.jar-Classes because those are guaranteed to be loaded with a
>> classloader who can look up SWT-Libary classes like
>> "org.eclipse.swt.internal.gtk.OS"
>>
>> BTW: Is the statement below really correct?
>>
>>  in JFX embeded into SWT scenarios JFX loads GTK primarily ...
>>
>> If you create an instance of FXCanvas SWT must have been loaded already
>> (You need to pass a parent Composite) so the native gtk-libs are there
>> already loaded.
>>
>> Tom
>>
>> On 22.11.16 14:51, Semyon Sadetsky wrote:
>>   
>>> Hello Kevin & David,
>>>
>>> Please review the fix for jfx9:
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8157002
>>>
>>> webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/
>>>
>>> In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
>>> version cannot be detected using the check for the loaded native library
>>> version.
>>>
>>> The fix proposes a way to detect if GTK version of swt.jar library is
>>> available on the classpath and tries to get the GTK version from SWT lib
>>> internal java classes.
>>>
>>> --Semyon
>>>
>>> 
>>
>>
>>   


-- 
Thomas Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
http://www.bestsolution.at/
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck


Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Tom Schindl
Hi,

Ok. I think I got it:

a) if SWT is loaded first (which is th 99% case I guess) then you are
   finding out that you need to run with gtk2 or gtk3 by finding out
   what gtk libs are already loaded (webref
http://cr.openjdk.java.net/~ssadetsky/8156491/webrev.00/)

b) if JavaFX is loaded before SWT but SWT is on the classpath then you
   try to findout what SWT will load and align with that (webref
http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.01/)

If that is correct all my concerns are addressed! There's one very
unlikely case:

a) one creates an OSGi-Application
b) one loads a JavaFX UI
c) loads an SWT-UI

Both webrefs won't address this case if not mistaken, but I have never
seen an application like that ;-)

Tom

On 23.11.16 13:32, Semyon Sadetsky wrote:
> On 11/23/2016 12:38 PM, Tom Schindl wrote:
> 
>>  From looking at the code I doubt your fix will work reliably in a
>> OSGi-Environment who is the Main deployment scenario for SWT and hence
>> FXCanvas!
>>
>> For sure you won't find the SWT-Library on the SystemClassloader and
>> whether you find it on the Thread-ContextClassloader is just gambling!
>>
>> The only area you for sure can detect the swt-library are the
>> swt-fx.jar-Classes because those are guaranteed to be loaded with a
>> classloader who can look up SWT-Libary classes like
>> "org.eclipse.swt.internal.gtk.OS"
> What is swt-fx.jar? Maybe you meant javafx-swt.jar?
>>
>> BTW: Is the statement below really correct?
>>
>>  in JFX embeded into SWT scenarios JFX loads GTK primarily ...
>>
>> If you create an instance of FXCanvas SWT must have been loaded already
>> (You need to pass a parent Composite) so the native gtk-libs are there
>> already loaded.
> That's right concern. Actually the fix covers the scenario when swt.jar
> is on the classpath and may be potentially loaded in future with another
> GTK version (which will cause the process crash). The rest scenarios are
> covered by the 8156491 fix which I just posted for review.
> 
> --Semyon
>>
>> Tom
>>
>> On 22.11.16 14:51, Semyon Sadetsky wrote:
>>> Hello Kevin & David,
>>>
>>> Please review the fix for jfx9:
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8157002
>>>
>>> webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/
>>>
>>> In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
>>> version cannot be detected using the check for the loaded native library
>>> version.
>>>
>>> The fix proposes a way to detect if GTK version of swt.jar library is
>>> available on the classpath and tries to get the GTK version from SWT lib
>>> internal java classes.
>>>
>>> --Semyon
>>>
>>
> 


-- 
Thomas Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
http://www.bestsolution.at/
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck


Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Kevin Rushforth

Hi Tom,

Would you have time to help us test this?

-- Kevin


Tom Schindl wrote:

From looking at the code I doubt your fix will work reliably in a
OSGi-Environment who is the Main deployment scenario for SWT and hence
FXCanvas!

For sure you won't find the SWT-Library on the SystemClassloader and
whether you find it on the Thread-ContextClassloader is just gambling!

The only area you for sure can detect the swt-library are the
swt-fx.jar-Classes because those are guaranteed to be loaded with a
classloader who can look up SWT-Libary classes like
"org.eclipse.swt.internal.gtk.OS"

BTW: Is the statement below really correct?

 in JFX embeded into SWT scenarios JFX loads GTK primarily ...

If you create an instance of FXCanvas SWT must have been loaded already
(You need to pass a parent Composite) so the native gtk-libs are there
already loaded.

Tom

On 22.11.16 14:51, Semyon Sadetsky wrote:
  

Hello Kevin & David,

Please review the fix for jfx9:

bug: https://bugs.openjdk.java.net/browse/JDK-8157002

webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/

In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
version cannot be detected using the check for the loaded native library
version.

The fix proposes a way to detect if GTK version of swt.jar library is
available on the classpath and tries to get the GTK version from SWT lib
internal java classes.

--Semyon





  


Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Semyon Sadetsky

On 11/23/2016 12:38 PM, Tom Schindl wrote:


 From looking at the code I doubt your fix will work reliably in a
OSGi-Environment who is the Main deployment scenario for SWT and hence
FXCanvas!

For sure you won't find the SWT-Library on the SystemClassloader and
whether you find it on the Thread-ContextClassloader is just gambling!

The only area you for sure can detect the swt-library are the
swt-fx.jar-Classes because those are guaranteed to be loaded with a
classloader who can look up SWT-Libary classes like
"org.eclipse.swt.internal.gtk.OS"

What is swt-fx.jar? Maybe you meant javafx-swt.jar?


BTW: Is the statement below really correct?

 in JFX embeded into SWT scenarios JFX loads GTK primarily ...

If you create an instance of FXCanvas SWT must have been loaded already
(You need to pass a parent Composite) so the native gtk-libs are there
already loaded.
That's right concern. Actually the fix covers the scenario when swt.jar 
is on the classpath and may be potentially loaded in future with another 
GTK version (which will cause the process crash). The rest scenarios are 
covered by the 8156491 fix which I just posted for review.


--Semyon


Tom

On 22.11.16 14:51, Semyon Sadetsky wrote:

Hello Kevin & David,

Please review the fix for jfx9:

bug: https://bugs.openjdk.java.net/browse/JDK-8157002

webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/

In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
version cannot be detected using the check for the loaded native library
version.

The fix proposes a way to detect if GTK version of swt.jar library is
available on the classpath and tries to get the GTK version from SWT lib
internal java classes.

--Semyon







[9] Review request for 8156491: Autodetect GTK version for JFX

2016-11-23 Thread Semyon Sadetsky

Hello Kevin & David,

Please review the fix for jfx9:

bug: https://bugs.openjdk.java.net/browse/JDK-8156491

webrev: http://cr.openjdk.java.net/~ssadetsky/8156491/webrev.00/

The fix implements the autodetect feature which protects JFX Glass 
toolkit from the crash caused by loading dynamic libs from different GTK 
versions into the same process on Linux platform.


--Semyon



Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Tom Schindl
>From looking at the code I doubt your fix will work reliably in a
OSGi-Environment who is the Main deployment scenario for SWT and hence
FXCanvas!

For sure you won't find the SWT-Library on the SystemClassloader and
whether you find it on the Thread-ContextClassloader is just gambling!

The only area you for sure can detect the swt-library are the
swt-fx.jar-Classes because those are guaranteed to be loaded with a
classloader who can look up SWT-Libary classes like
"org.eclipse.swt.internal.gtk.OS"

BTW: Is the statement below really correct?

 in JFX embeded into SWT scenarios JFX loads GTK primarily ...

If you create an instance of FXCanvas SWT must have been loaded already
(You need to pass a parent Composite) so the native gtk-libs are there
already loaded.

Tom

On 22.11.16 14:51, Semyon Sadetsky wrote:
> Hello Kevin & David,
> 
> Please review the fix for jfx9:
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8157002
> 
> webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/
> 
> In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
> version cannot be detected using the check for the loaded native library
> version.
> 
> The fix proposes a way to detect if GTK version of swt.jar library is
> available on the classpath and tries to get the GTK version from SWT lib
> internal java classes.
> 
> --Semyon
> 


-- 
Thomas Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
http://www.bestsolution.at/
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck


Re: [9] Review request for 8157002: Toggle gtk version if SWT used via FXCanvas

2016-11-23 Thread Semyon Sadetsky

Thanks, Doychin.

The webrev is updated: 
http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.01/


--Semyon


On 11/23/2016 10:15 AM, Doychin Bondzhev wrote:

Hi,

I think this line is incorrect:

if (ver < 2 && ver > 3) {

It should be || instead of &&

On 22.11.2016 г. 15:51 ч., Semyon Sadetsky wrote:

Hello Kevin & David,

Please review the fix for jfx9:

bug: https://bugs.openjdk.java.net/browse/JDK-8157002

webrev: http://cr.openjdk.java.net/~ssadetsky/8157002/webrev.00/

In JFX embeded into SWT scenarios JFX loads GTK primarily. So SWT GTK
version cannot be detected using the check for the loaded native library
version.

The fix proposes a way to detect if GTK version of swt.jar library is
available on the classpath and tries to get the GTK version from SWT lib
internal java classes.

--Semyon