[9] Code Review Request For 8157295: Encapsulate impl_ methods in Scene and KeyValue classes

2016-05-18 Thread Chien Yang

Hi Jonathan and Kevin,

Please review the proposed fix:

Webrev: https://bugs.openjdk.java.net/browse/JDK-8157295
JIRA: http://cr.openjdk.java.net/~ckyang/JDK-8157295/webrev.00/

Thanks,
- Chien


review: remove use of GetPropertyAction

2016-05-18 Thread David Hill


Kevin,

https://bugs.openjdk.java.net/browse/JDK-8157280
webrev:
http://cr.openjdk.java.net/~ddhill/8157280 


--
David Hill
Java Embedded Development

"A man's feet should be planted in his country, but his eyes should survey the 
world."
-- George Santayana (1863 - 1952)



Re: [9] Review request for 8157213: HiDPI support for Linux creates unnecessary dependency

2016-05-18 Thread Jim Graham

New updated webrevs with the new default value at:

http://cr.openjdk.java.net/~flar/JDK-8157213/webrev-01/

I also found a way to remove the schema from the Gentoo LiveCD environment and 
tested that way as well...

...jim

On 5/18/16 1:28 PM, Erik De Rijcke wrote:

Yes that seems to fix it.

On Wed, May 18, 2016 at 9:55 PM, Jim Graham  wrote:


Ah, this is probably me returning a -1 as an uint.  If you change the
"defval" used (line 167) in the call to query the property from -1 to 0
does it work as intended?

...jim


On 5/18/16 11:59 AM, Erik De Rijcke wrote:


I tested the patch.

Without the dependency I get a an enormous stage (I explicitly set it to
be 100x100). Debugging shows me the screen is
initialized with these values:

Screen:
ptr:0
adapter:0
depth:24
x:0
y:0
width:0
height:0
platformX:0
platformY:0
platformWidth:1680
platformHeight:946
visibleX:0
visibleY:0
visibleWidth:0
visibleHeight:0
platformScaleX:4.2949673E9
platformScaleY:4.2949673E9
outputScaleX:4.2949673E9
outputScaleY:4.2949673E9
resolutionX:0
resolutionY:0


I assume the scaling factor is not what it should be?

With the dependency installed the stage looks fine.

On Wed, May 18, 2016 at 5:52 AM, Jim Graham mailto:james.gra...@oracle.com>> wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8157213
webrev: http://cr.openjdk.java.net/~flar/JDK-8157213/webrev-00/

Details of what was fixed are listed in the bug report.  This will
hopefully fix all of the dependencies that Erik
ran into in his Gentoo environment...

...jim





Re: [9] Review request for 8157213: HiDPI support for Linux creates unnecessary dependency

2016-05-18 Thread Erik De Rijcke
Yes that seems to fix it.

On Wed, May 18, 2016 at 9:55 PM, Jim Graham  wrote:

> Ah, this is probably me returning a -1 as an uint.  If you change the
> "defval" used (line 167) in the call to query the property from -1 to 0
> does it work as intended?
>
> ...jim
>
>
> On 5/18/16 11:59 AM, Erik De Rijcke wrote:
>
>> I tested the patch.
>>
>> Without the dependency I get a an enormous stage (I explicitly set it to
>> be 100x100). Debugging shows me the screen is
>> initialized with these values:
>>
>> Screen:
>> ptr:0
>> adapter:0
>> depth:24
>> x:0
>> y:0
>> width:0
>> height:0
>> platformX:0
>> platformY:0
>> platformWidth:1680
>> platformHeight:946
>> visibleX:0
>> visibleY:0
>> visibleWidth:0
>> visibleHeight:0
>> platformScaleX:4.2949673E9
>> platformScaleY:4.2949673E9
>> outputScaleX:4.2949673E9
>> outputScaleY:4.2949673E9
>> resolutionX:0
>> resolutionY:0
>>
>>
>> I assume the scaling factor is not what it should be?
>>
>> With the dependency installed the stage looks fine.
>>
>> On Wed, May 18, 2016 at 5:52 AM, Jim Graham > > wrote:
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8157213
>> webrev: http://cr.openjdk.java.net/~flar/JDK-8157213/webrev-00/
>>
>> Details of what was fixed are listed in the bug report.  This will
>> hopefully fix all of the dependencies that Erik
>> ran into in his Gentoo environment...
>>
>> ...jim
>>
>>
>>


Re: Support for GTK 2 & 3 now in JFX

2016-05-18 Thread Erik De Rijcke
Hi,

I tried using the gtk3 broadway backend. This results in a segfault.

Current thread (0x7fe8cc0c2000):  JavaThread "GtkNativeMainLoopThread"
[_thread_in_native, id=21171, stack(0x7fe8abcad000,0x7fe8abdae000)]

Stack: [0x7fe8abcad000,0x7fe8abdae000],  sp=0x7fe8abdac1c0,
 free space=1020k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native
code)
C  [libX11.so.6+0x2d32a]  XInternAtom+0x2a

Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
j
 
com.sun.glass.ui.gtk.GtkApplication.staticScreen_getScreens()[Lcom/sun/glass/ui/Screen;+0
j  com.sun.glass.ui.Screen.initScreens()V+6
j  com.sun.glass.ui.Application.lambda$run$1(Ljava/lang/Runnable;)V+0
j  com.sun.glass.ui.Application$$Lambda$44.run()V+4
v  ~StubRoutines::call_stub
j  com.sun.glass.ui.gtk.GtkApplication._runLoop(Ljava/lang/Runnable;Z)V+0
j
 com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$8(Ljava/lang/Runnable;Z)V+7
j  com.sun.glass.ui.gtk.GtkApplication$$Lambda$48.run()V+12
j  java.lang.Thread.run()V+11
v  ~StubRoutines::call_stub

It looks like there are still some hard dependencies on X which will
obviously not work using a non X gtk back-end. (I verified the broadway
backend works by running the gtk3-demo app in my browser first).

On Wed, May 18, 2016 at 3:06 AM, Jim Graham  wrote:

> Hi Erik,
>
> I tried testing our build under a Gentoo LiveCD.  Even the minimal
> environment of the LiveCD has the org.gnome.desktop.interface schema
> defined so I'm not sure how minimal your environment was.  I did discover
> that, although the schema was there, it had a value of "0" for the
> scaling-factor which caused us to create bogus logical screen sizes that
> probably caused the exception you saw, so simply finding that schema and
> key is not enough for us to trust the value.  I'll add some protection for
> bad data from the key, but I'm curious to find out how minimal your install
> was and whether we should simply state our dependency, or if your
> configuration is likely to be common enough that we should protect against
> it in our code...?
>
> Also, what does:
>
> % gsettings get org.gnome.desktop.interface scaling-factor
>
> return for your system?  If you set it to "1", does FX come up fine?
>
> ...jim
>
>
> On 05/16/2016 01:01 PM, Jim Graham wrote:
>
>> These may both be related to the HiDPI fix instead.  I added a usage of
>> g_settings in that fix that went in on Friday.  It looks like I'll have
>> to check for the schema existing before I access it.
>>
>> Can you file a bug report?
>>
>> (On a side note, the call that grabs the schema does not have a "schema
>> not known" return value - if you ask for a schema that doesn't exist
>> they abort your application.  Seems kind of drastic, but the bug reports
>> that request that they simply return null were met with push back
>> because the developers couldn't imagine why anyone would want such a
>> thing (huh?) and later an attempt to return NULL on unrecognized schemas
>> and keys had to be backed out because of the huge disaster the NULL
>> values caused (and somehow ABORT'ing on behalf of the application is
>> better in that case?).  Unfortunately I don't see much sanity in those
>> APIs or any support for a case like this of "I'd like to get that value,
>> but if you haven't heard of it, that's fine as I have a backup plan".
>> The only way around this will be to enumerate all schemas and all keys
>> and see if the ones we want are found in the list - a rather extreme
>> workaround for bad error handling behavior in their APIs...)
>>
>>  ...jim
>>
>> On 05/16/2016 05:15 AM, Erik De Rijcke wrote:
>>
>>> I ran into several issues, however I'm not sure they are related to gtk3
>>>
>>> first of all it seems there is a dependency on:
>>> gsettings-desktop-schemas.
>>> I'm not sure how desirable this is (I did not have it installed,
>>> installing
>>> it fixed the error but I can image people not wanting to install it?)
>>>
>>> Second, and this seems unrelated to gtk(3?), the screen initialization
>>> fails (test was done using gentoo linux on virtualbox with windows 7 as
>>> host):
>>>
>>> Exception in thread "JavaFX Application Thread"
>>> java.lang.IllegalArgumentException: Both width and height must be >= 0
>>>
>>> at javafx.geometry.Rectangle2D.(Rectangle2D.java:104)
>>> at javafx.stage.Screen.nativeToScreen(Screen.java:152)
>>> at javafx.stage.Screen.updateConfiguration(Screen.java:88)
>>> at javafx.stage.Screen.checkDirty(Screen.java:82)
>>> at javafx.stage.Screen.getPrimary(Screen.java:185)
>>> at
>>>
>>> com.sun.javafx.tk.quantum.QuantumToolkit.initSceneGraph(QuantumToolkit.java:298)
>>>
>>> at
>>>
>>> com.sun.javafx.tk.quantum.QuantumToolkit.runToolkit(QuantumToolkit.java:340)
>>>
>>> at
>>>
>>> com.sun.javafx.tk.quantum.QuantumToolkit.lambda$startup$10(QuantumToolkit.java:257)
>>>
>>> at com.sun.glass.ui.Application.lambda$run$1(Application.java:155)
>>> at com.sun.glass.ui.gtk.GtkApplicati

Re: [9] Review request for 8157213: HiDPI support for Linux creates unnecessary dependency

2016-05-18 Thread Jim Graham
Ah, this is probably me returning a -1 as an uint.  If you change the "defval" used (line 167) in the call to query the 
property from -1 to 0 does it work as intended?


...jim

On 5/18/16 11:59 AM, Erik De Rijcke wrote:

I tested the patch.

Without the dependency I get a an enormous stage (I explicitly set it to be 
100x100). Debugging shows me the screen is
initialized with these values:

Screen:
ptr:0
adapter:0
depth:24
x:0
y:0
width:0
height:0
platformX:0
platformY:0
platformWidth:1680
platformHeight:946
visibleX:0
visibleY:0
visibleWidth:0
visibleHeight:0
platformScaleX:4.2949673E9
platformScaleY:4.2949673E9
outputScaleX:4.2949673E9
outputScaleY:4.2949673E9
resolutionX:0
resolutionY:0


I assume the scaling factor is not what it should be?

With the dependency installed the stage looks fine.

On Wed, May 18, 2016 at 5:52 AM, Jim Graham mailto:james.gra...@oracle.com>> wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8157213
webrev: http://cr.openjdk.java.net/~flar/JDK-8157213/webrev-00/

Details of what was fixed are listed in the bug report.  This will 
hopefully fix all of the dependencies that Erik
ran into in his Gentoo environment...

...jim




Re: [9] Review request for 8157213: HiDPI support for Linux creates unnecessary dependency

2016-05-18 Thread Erik De Rijcke
I tested the patch.

Without the dependency I get a an enormous stage (I explicitly set it to be
100x100). Debugging shows me the screen is initialized with these values:

Screen:
ptr:0
adapter:0
depth:24
x:0
y:0
width:0
height:0
platformX:0
platformY:0
platformWidth:1680
platformHeight:946
visibleX:0
visibleY:0
visibleWidth:0
visibleHeight:0
platformScaleX:4.2949673E9
platformScaleY:4.2949673E9
outputScaleX:4.2949673E9
outputScaleY:4.2949673E9
resolutionX:0
resolutionY:0


I assume the scaling factor is not what it should be?

With the dependency installed the stage looks fine.

On Wed, May 18, 2016 at 5:52 AM, Jim Graham  wrote:

> bug: https://bugs.openjdk.java.net/browse/JDK-8157213
> webrev: http://cr.openjdk.java.net/~flar/JDK-8157213/webrev-00/
>
> Details of what was fixed are listed in the bug report.  This will
> hopefully fix all of the dependencies that Erik ran into in his Gentoo
> environment...
>
> ...jim
>
>