[9] Code Review Request For 8164141: [Javadoc] Replace references of Stage with Window in the Window class

[Chien Yang]

Hi Kevin,

Please review the proposed doc fix:

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

Thanks,
- Chien

Re: Exception running FXMark with JavaFX 9

[dalibor topic]

On 16.08.2016 07:50, Felix Bembrick wrote:


How do I access PerformanceTracker in Java/JavaFX 9?


Please see http://openjdk.java.net/jeps/261

cheers,
dalibor topic

--
 Dalibor Topic | Principal Product Manager
Phone: +494089091214  | Mobile: +491737185961


ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

 Oracle is committed to developing
practices and products that help protect the environment

Exception running FXMark with JavaFX 9

[Felix Bembrick]

I am trying to port FXMark from Java 8 to Java 9 but encounter this
exception:

Caused by: java.lang.IllegalAccessError: class com.bembrick.fxmark.HUD (in
unnamed module @0x59223fd5) cannot access class
com.sun.javafx.perf.PerformanceTracker (in module javafx.graphics) because
module javafx.graphics does not export com.sun.javafx.perf to unnamed
module @0x59223fd5

How do I access PerformanceTracker in Java/JavaFX 9?

Thanks,

Felix

Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Alexander Zvegintsev]

Hi Alexander,

The fix looks good to me, except the wild card import change in 
FXCanvas.java(there is no need for a new patch, if it is the only issue 
found)



On 8/16/16 10:24 AM, Alexander Nyssen wrote:

You might even take the one I attached. I just recognized I still had some 
unused imports in the manual test case (I am still not familiar with IntelliJ).

Regards,
Alexander




Am 15.08.2016 um 19:26 schrieb Kevin Rushforth :

OK, I'll upload this revised version of the patch today.

-- Kevin


Alexander Nyssen wrote:

Hi Kevin,

please consider the following updated patch instead, which contains an 
additional null-check.

Regards
Alexander





Am 12.08.2016 um 16:04 schrieb Alexander Nyssen mailto:alexan...@nyssen.org>>:

Hi Kevin,

attached please find an initial patch for 
https://bugs.openjdk.java.net/browse/JDK-8161282 
.

The patch is not as minimal as I had hoped, as the EmbeddedSceneInterface had 
to be changed to differentiate between mouse and scroll events (while up to 
now, scroll events are handled as mouse events), but for me this seemed 
necessary to fix this issue properly. As a result, JFXPanel had to be adjusted 
as well to comply to the changes in the EmbeddedSceneInterface, while its 
behavior should not have changed.

As horizontal mouse events cannot be synthesized via Display.post(Event) yet 
(an open issue for SWT), I did not add an automated test, but instead added a 
manual one (FXCanvasMouseWheelEventsTest). Therefore, this patch does not 
depend on the patch I provided earlier for JDK-8160325.

Best Regards,
Alexander





=


--
Thanks,
Alexander.

Re: [PATCH] 8160325: Provide a public API to obtain the FXCanvas for an embedded scene.

[Alexander Zvegintsev]

Looks fine to me too.


On 8/16/16 1:45 AM, Kevin Rushforth wrote:
The 2016-08-12 revision looks fine to me, except for a missing space 
as noted in JBS (no need for a new patch if that is the only issue found).


While we wait for approval from the JDK 9 release team, we need 
another reviewer for this.


Alexander Z: can you take a look?

-- Kevin


Alexander Nyssen wrote:

Hi Kevin,

attached please find a revised patch that contains the corrections 
you requested. The patch is now applicable to the new module 
structure (modues with 'javafx.' prefix).


Regards,
Alexander





Am 12.08.2016 um 00:00 schrieb Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>>:




Alexander Nyssen wrote:

Hi Kevin,

thanks for your feedback. Please fin my comments inline.

   

Am 09.08.2016 um 03:10 schrieb Kevin Rushforth:

I uploaded the patch, reviewed it, and provided comments in the bug report. The 
short version is:

* The new API looks good

* There is a missing '@since 9' in the javadoc comments along with a few typos 
/ style issues
 

I will take care of it. Is there a style guide?
   


Not a current one. The ones I pointed out in the JBS issue were 
either grammatical or capitalization (other than the '@since' which 
is required for all new API).



* Rather than using reflection and setAccessible in the implementation, please 
add a public getHostContainer method to EmbeddedWindow (since it is an internal 
method, there is no concern with doing that -- it isn't API).
 

Such a getHost() method was already introduced to EmbeddedWindow as part of the 
patch (to obtain the HostContainer from it). The reflection code within 
getFXCanvas() is used to access the enclosing instance (i.e. the FXCanvas) of 
the HostContainer. We could only circumvent this by introducing a constructor 
to HostContainer and by passing in the enclosing FXCanvas instance explicitly 
(so we can query it via an explicit getter, which would have to be introduced 
in addition). Would you prefer that?
   


Ah, I see what you are doing now. There is an easier way, then. Just 
add a default (package) scope 'fxCanvas' variable in the inner class 
initialized to 'FXCanvas.this' and you will be able to access it 
directly, since an instance of an inner class can always get access 
to the instance of the enclosing class.


private class HostContainer implements HostInterface {
final FXCanvas fxCanvas = FXCanvas.this;
...

I updated the bug report with this and the other style issues. I 
haven't tested it yet, but other than the listed issues it looks 
very close to being done.


-- Kevin


   

Additionally, I requested JDK 9 release team approval for this. The approval 
process can proceed in parallel with your addressing the issues I raised.

— Kevin
 

Regards,
Alexander

   

Alexander Nyssen wrote:
 

Hi Kevin,

attached please find a revised patch. My comments are inlined.

   

Am 28.07.2016 um 18:03 schrieb Kevin Rushforth mailto:kevin.rushfo...@oracle.com>>:

Hi

Alexander Nyssen wrote:
 

Hi,

I have added my comments below:


   

Am 28.07.2016 um 17:22 schrieb Kevin Rushforth mailto:kevin.rushfo...@oracle.com>>:

I got the attachment, since Alexander also CCed me directly. I will attach it 
shortly.
 

Thanks!
   

Done.

 

I do have two comments on this:

1) We are past Feature Freeze, so all Enhancements need formal JDK 9 R-team 
approval [1][2]. In this case, the justification can be internal API that is no 
longer accessible in JDK 9 due to Jigsaw (I would be very reluctant to consider 
any other Enhancement request this late in the process), but I will need to 
look at it and then take it through the approval process, provided that I feel 
it is in scope.
 

I was not aware about this, but I would of course appreciate if it could be 
included (due to Jigsaw). Thanks for considering it at least.
   

I'll take a closer look tomorrow or Monday (no more time today). At first 
glance it seems like something reasonable to take forward.
 

That sounds promising. Thanks!

   

2) Some of the changes you list seem unrelated to this enhancement and are 
better done as separate issues (e.g., the rework of the SWTCursorsTest). Also, 
I am unconvinced of the need to force GTK 2; in fact it seems at odds with the 
work we have done with JEP 283 [3].
 

Well, the test case refactoring is somehow related, as I introduced the common 
SWT rule while introducing the second SWT test. However, I could provide it as 
a separate contribution if that was wished (and a JIRA issue was provided), but 
the rest of this contribution of course requires it as a prerequisite. If this 
enhancement could not be included in JDK 9, I would have to provide it as a 
separate contribution, as I would have to re-introduce FXCanvasTest in other 
succeeding bugfix contr

Re: Marking synthesized scroll events as such.

[Alexander Nyssen]

Having started to work on JDK-8143596 
 today, I have to provide an 
update on this: those PAN gesture events that are emulated by SWT should indeed 
be ignored by FXCanvas (as done by our FXCanvasEX implementation), because:

a) SCROLL gestures (START, SCROLL, FINISH) may otherwise not be properly 
recognized by JavaFX code (because they are always ‚disturbed‘ by intermediate 
synthesized mouse scroll events).
b) JavaFX does not emulate such events, so forwarding would result in a 
different behavior in case of the SWT integration (compared to JavaFX native).

Regards,
Alexander

> Am 16.08.2016 um 08:55 schrieb Alexander Nyssen :
> 
> Hi all,
> 
> as I am currently working on FXCanvas, there is one aspect I would like to 
> discuss, which is closely related to JDK-8161282 
>  (FXCanvas does not forward 
> horizontal mouse scroll events to the embedded scene) and JDK-8143596 
>  (FXCanvas does not forward 
> touch gestures to embedded scene), namely handling of synthesized scroll 
> events. That is, SWT synthesizes mouse wheel scroll events from PAN gesture 
> events, and these are forwarded to the embedded scene (the patch I provided 
> for JDK-8161282 does not change this behavior, it simply ensures horizontal 
> and vertical mouse wheel events are processed equally) while not being marked 
> as synthesized (unlike MouseEvent, ScrollEvent does not provide a 
> ‚synthesized‘ flag). JavaFX natively seems to do likewise with SWIPE events, 
> which seem to yield synthesized scroll events as well. 
> 
> Wouldn’t it be appropriate to introduce a synthesized flag and mark such kind 
> of events as being synthesized? Within the Eclipse GEF’s FXCanvasEx we have 
> sorted out scroll events synthesized form PAN events completely (and any 
> client code may do likewise without accessing internal API, so this is no 
> JIGSAW related issue), but I think it in general be nice if client code could 
> properly decide, whether it wants to deal with these events or rather handle 
> the native gesture events instead (as soon as JDK-8143596 is resolved this 
> would even be possible in an SWT-integrated scenario).
> 
> I would raise an issue via webbug for this, if my thoughts are shared.
> 
> Regards,
> Alexander
> 
> 

RE: Structuring CSS Stylesheets

[Daniel Glöckner]

Hi,

> -Original Message-
> From: Kevin Rushforth [mailto:kevin.rushfo...@oracle.com]
> Sent: Monday, August 15, 2016 6:02 PM
> To: David Grieve
> Cc: Daniel Glöckner; openjfx-dev@openjdk.java.net
> Subject: Re: Structuring CSS Stylesheets
> 
> One slight correction on how to contribute below.
> 
> 
> David Grieve wrote:
> >
> >
> > On 8/15/16 10:52 AM, Daniel Glöckner wrote:
>  We found the culprits by patching the JRE, adding some statistics
>  to SimpleSelector and CompoundSelector. I was wondering whether
>  there are easier ways but anyway, it works ;)
> >>> This sounds like some code that would be good to share with the
> >>> community. :)
> >> [DG] Sure thing. It's not too complicated and doesn't use external
> >> libs. Any hint where I could post it / paste it?
> > See https://wiki.openjdk.java.net/display/OpenJFX/Community for how to
> > contribute
> 
> That page is just a placeholder, and finding what you need in the sub-pages a
> bit tricky. See the following page for how to contribute:
> 
> http://openjdk.java.net/contribute/
> 
> -- Kevin
> 
> 
> >> need them, for example our UI component factory would add table.css
> >> to a TableView's list of stylesheets (tv.
> >> getStylesheets().add("/path/to/table.css").
> >> The global theme.css would be minimal and only define colors and fonts.
>  What do you think about this approach? Will this work nicely with
>  caching of
> >>> CSS styles in JavaFX?
> >>> I think if you are going to go this route, you might want to use
> >>> Region#getUserAgentStylesheets() which adds the styles as user-agent
> >>> styles.
> >>> But I don't think it will buy you much in terms of CSS performance.
> >> [DG] We also want to control / override the CSS of standard JavaFX
> >> controls like TreeTableView. Ideally we don't need to sub class them
> >> so we would need to use parent. getStylesheets().add(), right?
> > I doubt that getUserAgentStylesheets() or getStylesheets() is going to
> > have much impact. My guess is that having the stylesheets added to the
> > scene is going to be your best bet. I say this because the code that
> > does the style matching has to combine styles together from
> > Region#getUserAgentStylesheets() and Parent#getStylesheets(), whereas
> > the styles from scene stylesheets are already combined. You have to
> > think of these different sources of styles as sets of styles. When you
> > have Region or Parent stylesheets, you have to create a union of those
> > styles with the default user-agent stylesheets (e.g., caspian.css).
> > With just scene stylesheets, you have just one set (this isn't 100%
> > accurate, but close enough for this discussion).
OK. So if we're talking about performance it might be a good idea to add the 
CSS to the scene and make sure that all CSS selectors are efficient.
> >
> >>> If you the biggest bang for your buck relative to JavaFX CSS
> >>> performance, avoid style lookup and relative font sizes.
> >> [DG] Could you explain what you mean by "avoid style lookups"?
> > You know about styles like '-fx-base' used in caspian.css. You change
> > the color for -fx-base and the basic colors of the UI change. This
> > magic happens at runtime. So if I have a label in a cell in a table,
> > and it has a style "-fx-border-color: -fx-base", JavaFX will - at
> > runtime - try to resolve -fx-base into an actual color. It starts at
> > the leaf and looks tries to resolve -fx-base. If it can't resolve it,
> > it looks for a style in the parent node, and so on up the parent-chain
> > all the way to the root node. The worst case scenario, then, is that
> > there are no styles that resolve the value until you get to .root.
> >
> > This is what I mean by 'style lookups'.
> >
> > Its great stuff (the brainchild of Jasper Potts) because I can change
> > the look of my UI just by setting '-fx-base'. But if I were developing
> > a UI and I didn't care to let the users of my UI make such changes,
> > I'd go through and remove all the lookups in caspian.css (not trivial
> > because there are many many lookups - not just -fx-base). Or use a
> > pre-processor such as SASS or LESS.
> >
> > The same sort of lookup happens when you have an em (or other relative
> > size) because you need a font or a font-size to complete the
> > calculation. In most cases, the lookup for a font or font-size goes
> > all the way to .root, where it fails and falls back on
> > Font.getDefault(). But its a trade off since em sizes let your UI more
> > easily scale to different displays.
> >
Thanks a lot for the in-depth explanation. We're using a lot of style lookups 
in our CSS, mostly to define colors similar to -fx-base in your example. 
We will look at using a preprocessor as you suggested. This was anyway on our 
TODO list to be able to support user specific font sizes.

Re: [PATCH] 8161282: FXCanvas does not forward horizontal mouse scroll events to the embedded scene

[Alexander Nyssen]

You might even take the one I attached. I just recognized I still had some 
unused imports in the manual test case (I am still not familiar with IntelliJ).

Regards,
Alexander



> Am 15.08.2016 um 19:26 schrieb Kevin Rushforth :
> 
> OK, I'll upload this revised version of the patch today.
> 
> -- Kevin
> 
> 
> Alexander Nyssen wrote:
>> 
>> Hi Kevin,
>> 
>> please consider the following updated patch instead, which contains an 
>> additional null-check.
>> 
>> Regards
>> Alexander
>> 
>> 
>> 
>> 
>>> Am 12.08.2016 um 16:04 schrieb Alexander Nyssen >> >:
>>> 
>>> Hi Kevin,
>>> 
>>> attached please find an initial patch for 
>>> https://bugs.openjdk.java.net/browse/JDK-8161282 
>>> .
>>> 
>>> The patch is not as minimal as I had hoped, as the EmbeddedSceneInterface 
>>> had to be changed to differentiate between mouse and scroll events (while 
>>> up to now, scroll events are handled as mouse events), but for me this 
>>> seemed necessary to fix this issue properly. As a result, JFXPanel had to 
>>> be adjusted as well to comply to the changes in the EmbeddedSceneInterface, 
>>> while its behavior should not have changed.
>>> 
>>> As horizontal mouse events cannot be synthesized via Display.post(Event) 
>>> yet (an open issue for SWT), I did not add an automated test, but instead 
>>> added a manual one (FXCanvasMouseWheelEventsTest). Therefore, this patch 
>>> does not depend on the patch I provided earlier for JDK-8160325.
>>> 
>>> Best Regards,
>>> Alexander
>>> 
>>> 
>>> 
>>> 
>> 
>> =