Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling

[Robert Lichtenberger]

On Wed, 23 Mar 2022 08:15:47 GMT, Marius Hanl  wrote:

>> This fix respects a row factory, if present.
>> It will put the cell that is used to measure the column width as child below 
>> the row.
>> In that way the row's style will be used.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>  line 667:
> 
>> 665: if ((cell.getText() != null && !cell.getText().isEmpty()) 
>> || cell.getGraphic() != null) {
>> 666: tableRow.applyCss();
>> 667: cell.applyCss();
> 
> Just wondering: Is `cell.applyCss();` still needed here?

Right, this is no longer needed. I'll remove it.

-

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

Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling

[Robert Lichtenberger]

On Wed, 23 Mar 2022 08:17:38 GMT, Marius Hanl  wrote:

>> This fix respects a row factory, if present.
>> It will put the cell that is used to measure the column width as child below 
>> the row.
>> In that way the row's style will be used.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>  line 653:
> 
>> 651: tableSkin.getChildren().add(tableRow);
>> 652: tableRow.applyCss();
>> 653: ((SkinBase) tableRow.getSkin()).getChildren().add(cell);
> 
> I don't think this is a safe cast. Instead, you probably should do an 
> `instanceof SkinBase` check before

You're right. A tablerow may have been created that returns a custom skin which 
is not necessarily derived from SkinBase. 
However, since we do not have a getChildren() method in such a custom skin we 
will be unable to proceed. We could now abandon altogether (another return) but 
that mean that resizeColumnToFitContent no longer works at all in the presence 
of custom rows. So I think the best solution in that case would be to ignore 
the rowFactory and use a default `new TableRow<>` again to preserve at least 
the old behaviour in such cases.
I'll try and improve the patch.

-

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

Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling

[Robert Lichtenberger]

On Wed, 23 Mar 2022 08:19:41 GMT, Marius Hanl  wrote:

>> This fix respects a row factory, if present.
>> It will put the cell that is used to measure the column width as child below 
>> the row.
>> In that way the row's style will be used.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
>  line 650:
> 
>> 648: }
>> 649: Callback, TableRow> rowFactory = 
>> tv.getRowFactory();
>> 650: TableRow tableRow = rowFactory != null ? rowFactory.call(tv) 
>> : new TableRow<>();
> 
> When there is no row factory, we probably should just return like in line 
> 632-633?

Unlike cell factory, which always has to be present, the row factory is 
optional and in fact most tables will not have a row factory. If we return in 
that case, the algorithm will no longer work for these tables.
Compare with `javafx.scene.control.skin.TableViewSkin.createCell()`.

-

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

Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming [v3]

[Scott Palmer]

On Mar 23, 2022, at 8:20 AM, Kevin Rushforth  wrote:
> 
> On Tue, 22 Mar 2022 08:54:45 GMT, Johan Vos  wrote:
> 
>> I wonder if we can add some simple resources that allow testing the 
>> different protocols on different platforms?
> 
> I can't think of a good way to do that, especially for this feature, since it 
> requires a web server.

Just wondering why that’s an issue, specifically for HLS.  Java’s built-in web 
server, the old one at com.sun.net.httpserver.HttpServer (i.e. it wouldn’t need 
JDK 18), would work fine.

E.g. something not much more complicated than:

int port = ;
Path testFilesRoot = Paths.get("media");
HttpServer server = HttpServer.create(new InetSocketAddress(port), 0);
server.createContext("/hls", new HttpHandler() {
@Override
public void handle(HttpExchange exchange) throws IOException {
Path path = 
Paths.get(exchange.getRequestURI().getPath()).getFileName();
boolean isPlaylist = path.endsWith(".m3u8");
boolean isTS = path.endsWith(".ts");
Path assetPath = testFilesRoot.resolve(path);
byte[] data = Files.readAllBytes(assetPath);
exchange.getResponseHeaders().add("Content-Type", isPlaylist
? "application/x-mpegURL"
: isTS ? "video/MP2T" : "video/mp4");
exchange.sendResponseHeaders(200, data.length);
OutputStream os = exchange.getResponseBody();
os.write(data);
os.close();
}
});
server.start();

The content would all be short static test files, easily fitting in the byte 
array. You could grab something from a public domain source (just edit the 
paths in the .m3u8 files as needed)


Cheers,

Scott

Integrated: 8277309: Add support for H.265/HEVC to HTTP Live Streaming

[Alexander Matveev]

On Fri, 4 Feb 2022 11:24:48 GMT, Alexander Matveev  wrote:

> - Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. 
>  - Added support for elementary AAC streams without any container for audio 
> only streams.
>  - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder 
> cannot handle AAC elementary streams directly. DirectShow decoder works 
> without it.
>  - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will 
> be reloaded when fMP4 stream changes resolution. Dynamic format change did 
> not worked for these streams on Windows and Linux.

This pull request has now been integrated.

Changeset: 424aebae
Author:Alexander Matveev 
URL:   
https://git.openjdk.java.net/jfx/commit/424aebae8f28d0c75caa3281a3e5d6a7ede86a8a
Stats: 2690 lines in 29 files changed: 2384 ins; 137 del; 169 mod

8277309: Add support for H.265/HEVC to HTTP Live Streaming

Reviewed-by: kcr, arapte, jvos

-

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

Re: Question about fatal JavaFX crashes

[Daniel Peintner]

All,

Thank you very much for all the comments/suggestions.
I have been learning a lot about JVM C2 compilation the last couple of days.

BTW, in the meanwhile a similar bug has been reported with Debian Linux and
has been added to https://bugs.openjdk.java.net/browse/JDK-828338
.

I would like to briefly summarize what my solution is for the time being.
Since in my case stability is *much* more important than speed I used a
rather radical approach to prohibit C2 to do any *aggressive* optimization.

-XX:+TieredCompilation   (enable C1)
-XX:TieredStopAtLevel=1 (disable C2)

It seems somewhat similar to the out-dated "-client" option.
The application didn't really feel any slower. However, that certainly
depends heavily on the use case.

Note: I very much look forward to the day the GraalVM compiler gets the
default.

Hope this might be useful for others.

Thanks,

-- Daniel




On Fri, Mar 18, 2022 at 11:10 PM Scott Palmer  wrote:

> I wonder, could you do the opposite and force compilation to trigger the
> bug more consistently?
>
> Scott
>
> > On Mar 18, 2022, at 2:03 PM, Philip Race  wrote:
> >
> > I have at least seen  JIT compiler crashes like this in other unrelated
> cases ..
> >
> > in theory you can use an option like
> >
> > -XX:CompileCommand=exclude,javafx/scene/control/TableView$5::onChanged
> >
> > although I am very unsure about the syntax for the last part of it
> especially with what looks like
> > some anonymous method or lambda expression. I can almost guarantee that
> my example is wrong.
> >
> > Excluding the whole of TableView might be easier to specify but will
> slow you down a lot.
> >
> > -phil.
> >
> >
> >> On 3/18/22 10:37 AM, Kevin Rushforth wrote:
> >> Thanks, Phil. That was my take as well. I don't see how this can be a
> JavaFX bug given where it is crashing. FWIW, I haven't ever seen anything
> like this.
> >>
> >> -- Kevin
> >>
> >>
> >>> On 3/18/2022 10:25 AM, Philip Race wrote:
> >>> I think it is probable that this is a hotspot VM problem in the C2 JIT
> compiler code.
> >>>
> >>> I've moved your bug report to hotspot :
> https://bugs.openjdk.java.net/browse/JDK-8283386
> >>> The interesting question isn't about which version of FX used to work.
> >>> It is what was the last working version of the JDK.
> >>> It looks a bit like a JDK 17 bug from the evidence so far so if
> >>> you were running FX 17 on JDK 17 GA you maybe have picked up a later
> update release  of JDK 17 along with FX 18 ??
> >>>
> >>> Regardless this doesn't look like an FX bug.
> >>> But unless you can actually provide a test case, or, by luck the
> hotspot folks recognise the issue,
> >>> I don't know what can/will happen.
> >>>
> >>> -phil.
> >>>
> >>> On 3/18/22 7:08 AM, Daniel Peintner wrote:
>  Hi John, all,
> 
>  Thanks for your detailed reply.
>  I submitted a bug report with detailed information.
> 
>  - Run a different Java version
>  I tried different versions and vendors with the same result.
>  * OpenJDK 17.0.1
>  * Zulu 17.0.2
>  * I wanted to check also JDK18-ea but gradle does not yet support it
> 
> 
> > - Try switching to a different GC
> > - Use different VM options (are you using anything special?)
> > - Anything else that is not often used, non-standard or experimental,
> > try going to a more common setup
> >
>  I use no specific setting, all is default.
>  I have been reading that the -client flag might help in some cases but
>  unfortunately this flag is no longer taken into account for 64bit
> JVMS.
> 
>  Thanks,
> 
>  -- Daniel
> 
> 
> 
> 
> > --John
> >
> > On 18/03/2022 09:43, Daniel Peintner wrote:
> >> Hello,
> >>
> >> I take the liberty to ask on the email reflector if there are other
> > people
> >> with similar problems.
> >>
> >> Since updating my application to JavaFX18 I get random fatal
> crashes.
> >> Unfortunately it is not predictable but after some time the app
> crashes
> >> with "EXCEPTION_ACCESS_VIOLATION".
> >>
> >> The hs_error_pidX reports the following.
> >>
> >>
> >> ---  T H R E A D  ---
> >>
> >> Current thread (0x016be9c9b410):  JavaThread "C2
> CompilerThread0"
> >> daemon [_thread_in_native, id=3068,
> >> stack(0x00393e80,0x00393e90)]
> >>
> >> Current CompileTask:
> >> C2:10515286 18185   4
> >   javafx.scene.control.TableView$5::onChanged
> >> (1049 bytes)
> >>
> >>
> >> Am I the only one seeing this?
> >> I am not sure if this relates to changing JavaFX 17 to 18 or
> whether it
> >> makes it just more likely.
> >>
> >> It seems to be related to
> "javafx.scene.control.TableView$5::onChanged"
> >> since all crashes show this line.
> >>
> >> Having said that, I cannot provide a test-case 

Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming [v3]

[Kevin Rushforth]

On Tue, 22 Mar 2022 08:54:45 GMT, Johan Vos  wrote:

> I wonder if we can add some simple resources that allow testing the different 
> protocols on different platforms?

I can't think of a good way to do that, especially for this feature, since it 
requires a web server.

-

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

Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming [v3]

[Kevin Rushforth]

On Fri, 18 Mar 2022 02:20:11 GMT, Alexander Matveev  
wrote:

>> - Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. 
>>  - Added support for elementary AAC streams without any container for audio 
>> only streams.
>>  - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder 
>> cannot handle AAC elementary streams directly. DirectShow decoder works 
>> without it.
>>  - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will 
>> be reloaded when fMP4 stream changes resolution. Dynamic format change did 
>> not worked for these streams on Windows and Linux.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8277309: Add support for H.265/HEVC to HTTP Live Streaming [v3]

Marked as reviewed by kcr (Lead).

-

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

Re: RFR: 8281723: Spinner with split horizontal arrows and a border places right arrow incorrectly [v2]

[Marius Hanl]

On Wed, 9 Mar 2022 07:48:53 GMT, John Hendrikx  wrote:

>> I added a test case for `SpinnerSkin` that checks the arrow positioning.
>> 
>> While adding the tests I discovered more problems with the positioning aside 
>> from the one mentioned in the JBS ticket.
>> 
>> 1) Vertical split arrow placement also forgot to take the padding into 
>> account while placing the decrement arrow button -- I've taken the liberty 
>> to fix that problem as well in the same PR.
>> 
>> 2) When arrows are placed next to each other either on the right or left, 
>> the arrow widths are not normalized to be the width of the widest arrow.  
>> All other placements will normalize either the width or height, except for 
>> these two.  Specifically, when the arrows are **split** on the left and 
>> right they **are** normalized to the same width.  
>> 
>> For point 2, here is the problem illustrated with actual widths on left and 
>> layout result on right:
>> 
>>  [ <- ] [ -> ] [ spinner ]   -->  [ <- ] [ -> ] [ 
>> spinner ]
>>  [ spinner ] [ <- ] [ -> ]   -->  [ spinner ] [ <- ] 
>> [ -> ]
>> 
>> While for split horizontal it does normalize the width to that of the widest 
>> arrow, and so layout becomes:
>> 
>>  [ <- ] [ spinner ] [ -> ]   -->  [ <- ] [ spinner ] 
>> [   ->   ]
>> 
>> While I'm here I could fix this as well, and adjust the test case to match.
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update copyright year

Fix + Tests look good to me

-

Marked as reviewed by mhanl (Author).

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

Re: RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling

[Marius Hanl]

On Wed, 16 Mar 2022 08:20:59 GMT, Robert Lichtenberger  
wrote:

> This fix respects a row factory, if present.
> It will put the cell that is used to measure the column width as child below 
> the row.
> In that way the row's style will be used.

The approach looks good. I left some comments and questions

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
 line 650:

> 648: }
> 649: Callback, TableRow> rowFactory = 
> tv.getRowFactory();
> 650: TableRow tableRow = rowFactory != null ? rowFactory.call(tv) 
> : new TableRow<>();

When there is no row factory, we probably should just return like in line 
632-633?

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
 line 653:

> 651: tableSkin.getChildren().add(tableRow);
> 652: tableRow.applyCss();
> 653: ((SkinBase) tableRow.getSkin()).getChildren().add(cell);

I don't think this is a safe cast. Instead, you probably should do an 
`instanceof SkinBase` check before

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java
 line 667:

> 665: if ((cell.getText() != null && !cell.getText().isEmpty()) || 
> cell.getGraphic() != null) {
> 666: tableRow.applyCss();
> 667: cell.applyCss();

Just wondering: Is `cell.applyCss();` still needed here?

modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java
 line 259:

> 257: /**
> 258:  * @test
> 259:  * @bug 8251480 Row style must affect the required column width

This annotations are not needed, although they do not hurt (just a note)

-

Changes requested by mhanl (Author).

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

Re: RFR: 8277309: Add support for H.265/HEVC to HTTP Live Streaming [v3]

[Johan Vos]

On Fri, 18 Mar 2022 02:20:11 GMT, Alexander Matveev  
wrote:

>> - Added support for fragmented MP4 with HEVC/H.265, H.264 and AAC. 
>>  - Added support for elementary AAC streams without any container for audio 
>> only streams.
>>  - Added "aacparse" plugin from GStreamer. Required on Linux, since decoder 
>> cannot handle AAC elementary streams directly. DirectShow decoder works 
>> without it.
>>  - DirectShow H.264 decoder on Windows and H.265/H.264 decoder on Linux will 
>> be reloaded when fMP4 stream changes resolution. Dynamic format change did 
>> not worked for these streams on Windows and Linux.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8277309: Add support for H.265/HEVC to HTTP Live Streaming [v3]

Marked as reviewed by jvos (Reviewer).

-

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

Re: Promote addEventHandler/removeEventHandler to EventTarget interface

[Michael Strauß]

I think defaulting to UnsupportedOperationException is a good choice.
This shouldn't break existing usages of 3rd party implementations,
since they wouldn't be calling those APIs anyway.


On Fri, Mar 18, 2022 at 10:55 PM John Hendrikx  wrote:
>
> [...]
> EventTarget is already public API, and so there might be 3rd party
> implementations.  This means that the methods added to the EventTarget
> interface must be default methods. It would be super if these default
> implementations would just work out of the box, but that would require
> exposing the internal class EventHandlerManager (and adding a
> `getEventHandlerManager` to the `EventTarget` interface).  If that's not
> realistic, then initially the default implementations would have to
> throw UnsupportedOperationException.
>
> --John
>