Re: Glass Robot and getSCreenCapture
Hi David, sorry to keep you waiting with an answer from Embedded SQE. I assume there will be no major impact on SQE since as you told and it is true, most golden image tests we have are based on a window content. Anyway from what I understood your implementation will be easily rolled back if we reveal some unexpected impact on tests, right? 26.03.2014 23:03, David Hill ?: On 3/26/14, Mar 26, 12:53 PM, Stephen F Northover wrote: I don't like "implied" contract code either but I also don't like exceptions for cases like this. I would prefer that we return zero for pixels that are unspecified as this seems better than testing screen bounds (which can get you in trouble on multi-display monitors). Anyway, to fix this involves writing a test case that we can run on all platforms in a multi-monitor scenario. Also, the primary monitor can be on either the left or the right and this might affect the result. It's easier to fix this in Java code by testing screen bounds as you were doing and throw the exception. Since this is not API and we need to move on, then we could do this (and possibly break SQE). Alternately, we can construct the test cases, see if the platforms/glass already return zero and assess where to go next. Whatever happens, we need a test case. Is there one in the JIRA? If so, I can run it on the desktop platforms and let you know the results for the current code base. As much as I don't like it - I am no longer sure there is a reasonable pre-test that can be done. I suggested a four corner test, but in the case of two adjacent screens of different heights, it is reasonable to see that you could ask for bounds that would put 3 corners in, and one out (hopefully the asci art will work): +++ ||| ||| ||| ++| || ++ For a one shot screen capture - we would pass in top left and width and height to make the bottom right. Currently this should work on Mac I am told, though what is in the out of bounds pixels is not known. And if we added a third "tall" screen to the left, life gets even more complicated :-( I was hoping to simplify the native impl for ARM by making it impossible to have an out of bounds pixel. This thought was in line with other API - check for valid values before calling the impl. We still could, but in the above case, there would not be a way to get all the screen in one shot. I really don't think we should be having a major impact on SQE, as I would think that most golden image tests will be based on checking "known" things - like the content of a window. But ... I have erred recently in the past on this subject... :-) The test case I have been using is in HelloSanity. It is "well behaved". It is only one of 2 apps in our repo that perform any screen captures (an the other was used as the basis for HelloSanity). There are some uses of getPixel(x,y) which is a variation. I found the problem in the ARM code by inspection, and have yet to write a reasonable test app that includes the aprox 6-8 variations of overlap (ie, full subset, off left, off right, completely missing up, .) I certainly can throw together something that will try some of the variations to see if we fail on other platforms. Given my current understanding of the problem though, I really don't see how a pre-verification of the bounds can be done. -- Best regards, Stas Smirnov Stas Smirnov | Java Embedded Phone: +7 812 3346130 | Mobile: +7 921 9262241 Oracle Development SPB, LLC 10th Krasnoarmeyskaya 22A, St. Petersburg, 190103, Russia
Re: Adding GStreamer plugins
Unfortunately I seem to have become a bit stuck at step one! I'm probably missing a few things that are rather obvious, but haven't really got much experience at all in this area, so am relying on haphazard Googling and a bit of guesswork! It may well be that updating the framework is a bit beyond my skillset for the time I have available at the moment, but I'll keep pushing on if anyone's willing to give me a bit of guidance on it. Firstly libffi - I've downloaded that, put it in the 3rd_party folder, and have run "./configure", "make", then "make install" (all under cygwin.) That seems fine; so I've now got "libffi.a" (as well as "cygffi-6.dll") in "3rd_party\libffi\i686-pc-cygwin\.libs". I've then grabbed glib 2.40, placed that in the glib folder, and attempted the same process - this first complained about a lack of libiconv, so I grabbed that and set the appropriate flags to point to it, and now I get the following: *configure: error: in `/cygdrive/c/Users/Michael/Documents/JFX8/modules/media/src/main/native/gstreamer/3rd_party/glib/glib-2.28.8':* *configure: error: The pkg-config script could not be found or is too old. Make sure it is in your PATH or set the PKG_CONFIG environment variable to the full path to pkg-config.* *Alternatively, you may set the environment variables LIBFFI_CFLAGS and LIBFFI_LIBS to avoid the need to call pkg-config.* I'm assuming this is something to do with the fact it can't find libffi, but I'm unaware of how to tell it that libffi is in the given folder! I'm also assuming that calling config / make etc. manually is the way to go for the time being - I think I'd be at even more of a loss trying to integrate it into the gradle build script. Thanks, Michael On 26 March 2014 12:43, Kirill Kirichenko wrote: > Michael, > > > On 26.03.2014 04:11, Michael Berry wrote: > >> Kirill - I think I'll take your suggestion next and start looking at >> upgrading the existing native components to the latest version of >> GStreamer >> before I look at adding any more plugins, that would seem to make sense. >> Have you any pointers in how best to approach this? >> > No pointer at all. I have it my my head. And it's about time to pass this > experience before I forget it =) > The thing is I did it once and it took me ~ 2 months. > > Here is the brief plan that you need to keep: > [1] Start with the lower lever. It's glib. Linux doesn't need any glib > update - we use the system glib. So glib update is necessary for Win/Mac. > [1.1] Latest glib has plenty of dependencies on other 3-rd party > libraries. But there is one that's mandatory - libFFI. Fortunately Oracle > has approval to use libFFI in it's products. But this probably isn't > necessary for OpenJFX. So at you first step you need to bring libFFI > sources, place them in rt/modules/media/src/main/ > native/gstreamer/3rd_party/libffi and make sure it builds on Windows and > Mac producing lib/a for static linking. You can probably make dll/dylib > instead but I don't think it's necessary. > [1.2] Take the latest glib 2.38 or even 2.40. Place them in > rt/modules/media/src/main/native/gstreamer/3rd_party/glib > Note that you don't need all sources/headers. But to remove precisely > what's redundant you should first compile gstreamer/plugins. > Here you make sure you can compile and build glib-lite.dll/libglib-lite. > dylib > Having done 1.2 you should be able to run media component with the new > glib-lite.dll. If it runs then you're done with glib upgrade. > It's important to apply fixes that we made in glib to the newer glib > library. You can find them by grepping for GSTREAMER_LITE in > sources/headers. > > [2] GStreamer update. > [3] Oracle plugins compilation/update. This step will also be necessary > because 0.10.35 API is different from 1.0. For Example GstBuffer that we > extensively use has incompatible APIs. > > I won't get deeper into details for [2] and [3] now. Let's just handle [1] > and then continue. > >
hg: openjfx/8u-dev/rt: 2 new changesets
Changeset: 335ad99f854f Author:kcr Date: 2014-03-26 14:40 -0700 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/335ad99f854f RT-35019: [3D] NPE in NGShape.renderContent when drawing empty shapes Reviewed-by: flar ! modules/graphics/src/main/java/com/sun/javafx/sg/prism/NGShape.java + tests/system/src/test/java/test3d/RT35019Test.java Changeset: 755b1d48c070 Author:kcr Date: 2014-03-26 14:40 -0700 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/755b1d48c070 [TEST-ONLY] RT-31190: RT_5239Test uses java.awt.Color instead of Prism color ! modules/graphics/src/test/java/com/sun/scenario/effect/rt_5239/RT_5239Test.java
hg: openjfx/8u-dev/rt: 2 new changesets
Changeset: bd741e03a5d2 Author:jgiles Date: 2014-03-27 09:16 +1300 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/bd741e03a5d2 RT-36392: [SplitPane] Cannot lookup children inside a SplitPane ! modules/controls/src/main/java/com/sun/javafx/scene/control/skin/SplitPaneSkin.java ! modules/controls/src/test/java/javafx/scene/control/SplitPaneTest.java Changeset: cf8541657afc Author:jgiles Date: 2014-03-27 09:17 +1300 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/cf8541657afc RT-36316: [Accessibility] Improve TreeView virtualization on Windows ! modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeViewSkin.java ! modules/controls/src/main/java/com/sun/javafx/scene/control/skin/VirtualFlow.java
hg: openjfx/2u/dev/rt: Added tag 2.2.60-b12 for changeset c1541c2d6a96
Changeset: ad9fa39517da Author:hudson Date: 2014-03-26 08:12 -0700 URL: http://hg.openjdk.java.net/openjfx/2u/dev/rt/rev/ad9fa39517da Added tag 2.2.60-b12 for changeset c1541c2d6a96 ! .hgtags
Re: Glass Robot and getSCreenCapture
> For a one shot screen capture - we would pass in top left and width and > height to make the bottom right. > Currently this should work on Mac I am told, though what is in the out of > bounds pixels is not known. > And if we added a third "tall" screen to the left, life gets even more > complicated :-( When pressing Command+Shift+3, the Mac creates separate images for each display. -DrD-
Re: Glass Robot and getSCreenCapture
On 3/26/14, Mar 26, 12:53 PM, Stephen F Northover wrote: I don't like "implied" contract code either but I also don't like exceptions for cases like this. I would prefer that we return zero for pixels that are unspecified as this seems better than testing screen bounds (which can get you in trouble on multi-display monitors). Anyway, to fix this involves writing a test case that we can run on all platforms in a multi-monitor scenario. Also, the primary monitor can be on either the left or the right and this might affect the result. It's easier to fix this in Java code by testing screen bounds as you were doing and throw the exception. Since this is not API and we need to move on, then we could do this (and possibly break SQE). Alternately, we can construct the test cases, see if the platforms/glass already return zero and assess where to go next. Whatever happens, we need a test case. Is there one in the JIRA? If so, I can run it on the desktop platforms and let you know the results for the current code base. As much as I don't like it - I am no longer sure there is a reasonable pre-test that can be done. I suggested a four corner test, but in the case of two adjacent screens of different heights, it is reasonable to see that you could ask for bounds that would put 3 corners in, and one out (hopefully the asci art will work): +++ ||| ||| ||| ++| || ++ For a one shot screen capture - we would pass in top left and width and height to make the bottom right. Currently this should work on Mac I am told, though what is in the out of bounds pixels is not known. And if we added a third "tall" screen to the left, life gets even more complicated :-( I was hoping to simplify the native impl for ARM by making it impossible to have an out of bounds pixel. This thought was in line with other API - check for valid values before calling the impl. We still could, but in the above case, there would not be a way to get all the screen in one shot. I really don't think we should be having a major impact on SQE, as I would think that most golden image tests will be based on checking "known" things - like the content of a window. But ... I have erred recently in the past on this subject... :-) The test case I have been using is in HelloSanity. It is "well behaved". It is only one of 2 apps in our repo that perform any screen captures (an the other was used as the basis for HelloSanity). There are some uses of getPixel(x,y) which is a variation. I found the problem in the ARM code by inspection, and have yet to write a reasonable test app that includes the aprox 6-8 variations of overlap (ie, full subset, off left, off right, completely missing up, .) I certainly can throw together something that will try some of the variations to see if we fail on other platforms. Given my current understanding of the problem though, I really don't see how a pre-verification of the bounds can be done. -- David Hill Java Embedded Development "Experience is a hard teacher because she gives the test first, the lesson afterwards." -- Vernon Sanders Law
[8u Review Request] RT-35864: Button's events are not working in a ListView
Jonathan, Please review: http://cr.openjdk.java.net/~dgrieve/RT-35864/webrev.00 A summary of the changes is in: https://javafx-jira.kenai.com/browse/RT-35864
hg: openjfx/8u-dev/rt: 2 new changesets
Changeset: 91fdf1709eae Author:hudson Date: 2014-03-26 11:23 -0700 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/91fdf1709eae Added tag 8u20-b07 for changeset eee373287ad8 ! .hgtags Changeset: 5f8012b58540 Author:kcr Date: 2014-03-26 11:35 -0700 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/5f8012b58540 Automated merge with ssh://jfxsrc.us.oracle.com//javafx/8u/master/jfx/rt - apps/samples/Ensemble8/src/samples/java/ensemble/samples/language/swing/ProcessListener.java
hg: openjfx/8u-dev/rt: RT-31075: Date picker fails with LocalDate.MAX and LocalDate.MIN values.
Changeset: 0f1a42edda54 Author:leifs Date: 2014-03-26 10:11 -0700 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/0f1a42edda54 RT-31075: Date picker fails with LocalDate.MAX and LocalDate.MIN values. ! modules/controls/src/main/java/com/sun/javafx/scene/control/skin/DatePickerContent.java
[8u6] Review request: RT-36387: [IMX, Lens] Black rectangle seen instead of cursor on first run after reboot
Daniel, Rafi, Please review the simple fix for https://javafx-jira.kenai.com/browse/RT-36387 - [IMX, Lens] Black rectangle seen instead of cursor on first run after reboot Diff is in the jira. Thanks, Lisa
Re: Glass Robot and getSCreenCapture
On 2014-03-26 12:22 PM, David Hill wrote: On 3/26/14, Mar 26, 10:59 AM, Stephen F Northover wrote: Hi David, Sorry to be getting to this so late. An uninitialized pixel is normally considered to be black. If you throw an exception, clients will need to either catch the exception or perform the same test that you are performing before they call the "API". Since this is not pubic API, no client will be affected so even if we make the change to throw the exception and then decide not to do this later, we can change it. What is happening now? Who is being affected by this bug? Is it easy to change the implementations to return black? This seems better to me than throwing the exception. Why ? Because I happened to have to fix some code and saw that what we had in ARM was broken, or at least made some poor assumptions around out of bounds pixels. I really, really don't like "implied" contract code, and robot screen capture is certainly implied. My preference in javadoc, even on internal API that outlines the expected behavior, instead of having to read impl code on platforms I am not familiar with. The main item still is: * what does an out of bounds pixel look like. The current answer is - depends. Certainly on ARM some of the existing code would return one of, uninitialized, or 0. Note that 0 is not always the same as black (0xff00). I don't like "implied" contract code either but I also don't like exceptions for cases like this. I would prefer that we return zero for pixels that are unspecified as this seems better than testing screen bounds (which can get you in trouble on multi-display monitors). Anyway, to fix this involves writing a test case that we can run on all platforms in a multi-monitor scenario. Also, the primary monitor can be on either the left or the right and this might affect the result. It's easier to fix this in Java code by testing screen bounds as you were doing and throw the exception. Since this is not API and we need to move on, then we could do this (and possibly break SQE). Alternately, we can construct the test cases, see if the platforms/glass already return zero and assess where to go next. Whatever happens, we need a test case. Is there one in the JIRA? If so, I can run it on the desktop platforms and let you know the results for the current code base. I sought to make the impl code easier by denying a call that would be out of bounds. I am finding that this may be difficult or even impossible, when you glue two screens of different sizes. Thanks to Anthony educating me on how this works. I did glance through the other impls code and I did not see any specific handling of out of bounds pixels. Anthony says that handling was done by the the underlying system calls. As I say, if we throw the exception, then we will only break ourselves, not clients of FX API. Have we ensured that the exception will not break SQE tests. Ensured ? Not sure that is possible except by running the SQE tests with a restriction in place. I have asked for a reading from SQE but have not heard back yet. But this discussion should be happening anyway, as we discuss what should or should not be done. Dave Again, sorry to be getting to this so late and apologies if all of this has been discussed in another thread that I missed, Steve On 2014-03-25 2:46 PM, David Hill wrote: On 3/21/14, Mar 21, 12:53 PM, David Hill wrote: Having heard a little feedback, here is my proposal in the form of a review request :-) My recommendation would be modify the JavaDoc contract to specify that the x,y and x+width, y+height must be within the screen bounds, with an IllegalArgumentException if out of bounds. This would be checked in Robot, prior to calling the native impls. This code is "internal" API, so I expect the real impact would be on SQE. I really like the idea of adding a bounds restriction - so that the requested bounds must be within the Screen. It seems the simplest solution to my issue of handling the odd edge case of out of bound pixels, with the least likely impact. This means that existing code in the implementations are not affected. I suspect that there will we little if any impact on SQE tests, given that most of us would avoid asking for a screen capture with undefined pixels. I do expect that we will encounter a few exceptions to this when tests are run on smaller displays (like embedded). I also added bounds checking to Robot.getPixelColor() for consistency, and because Embedded passes this call through to common code for screen capture. I did a grep through the JavaFX code base, and don't see any JavaFX use cases. I expect any "golden image" test code could be affected. I separated out this internal API changes from my embedded changes so we have a clear and easy thing to review. Jira: https://javafx-jira.kenai.com/browse/RT-36382 Patch: is inline in the Jira, but also here: diff -r bb72bd
Re: Glass Robot and getSCreenCapture
On 3/26/14, Mar 26, 10:59 AM, Stephen F Northover wrote: Hi David, Sorry to be getting to this so late. An uninitialized pixel is normally considered to be black. If you throw an exception, clients will need to either catch the exception or perform the same test that you are performing before they call the "API". Since this is not pubic API, no client will be affected so even if we make the change to throw the exception and then decide not to do this later, we can change it. What is happening now? Who is being affected by this bug? Is it easy to change the implementations to return black? This seems better to me than throwing the exception. Why ? Because I happened to have to fix some code and saw that what we had in ARM was broken, or at least made some poor assumptions around out of bounds pixels. I really, really don't like "implied" contract code, and robot screen capture is certainly implied. My preference in javadoc, even on internal API that outlines the expected behavior, instead of having to read impl code on platforms I am not familiar with. The main item still is: * what does an out of bounds pixel look like. The current answer is - depends. Certainly on ARM some of the existing code would return one of, uninitialized, or 0. Note that 0 is not always the same as black (0xff00). I sought to make the impl code easier by denying a call that would be out of bounds. I am finding that this may be difficult or even impossible, when you glue two screens of different sizes. Thanks to Anthony educating me on how this works. I did glance through the other impls code and I did not see any specific handling of out of bounds pixels. Anthony says that handling was done by the the underlying system calls. As I say, if we throw the exception, then we will only break ourselves, not clients of FX API. Have we ensured that the exception will not break SQE tests. Ensured ? Not sure that is possible except by running the SQE tests with a restriction in place. I have asked for a reading from SQE but have not heard back yet. But this discussion should be happening anyway, as we discuss what should or should not be done. Dave Again, sorry to be getting to this so late and apologies if all of this has been discussed in another thread that I missed, Steve On 2014-03-25 2:46 PM, David Hill wrote: On 3/21/14, Mar 21, 12:53 PM, David Hill wrote: Having heard a little feedback, here is my proposal in the form of a review request :-) My recommendation would be modify the JavaDoc contract to specify that the x,y and x+width, y+height must be within the screen bounds, with an IllegalArgumentException if out of bounds. This would be checked in Robot, prior to calling the native impls. This code is "internal" API, so I expect the real impact would be on SQE. I really like the idea of adding a bounds restriction - so that the requested bounds must be within the Screen. It seems the simplest solution to my issue of handling the odd edge case of out of bound pixels, with the least likely impact. This means that existing code in the implementations are not affected. I suspect that there will we little if any impact on SQE tests, given that most of us would avoid asking for a screen capture with undefined pixels. I do expect that we will encounter a few exceptions to this when tests are run on smaller displays (like embedded). I also added bounds checking to Robot.getPixelColor() for consistency, and because Embedded passes this call through to common code for screen capture. I did a grep through the JavaFX code base, and don't see any JavaFX use cases. I expect any "golden image" test code could be affected. I separated out this internal API changes from my embedded changes so we have a clear and easy thing to review. Jira: https://javafx-jira.kenai.com/browse/RT-36382 Patch: is inline in the Jira, but also here: diff -r bb72bd2fa889 modules/graphics/src/main/java/com/sun/glass/ui/Robot.java --- a/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue Mar 25 14:21:26 2014 -0400 +++ b/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue Mar 25 14:41:37 2014 -0400 @@ -144,9 +144,20 @@ protected abstract int _getPixelColor(int x, int y); /** * Returns pixel color at specified screen coordinates in IntARGB format. + * + * If the requested pixel is not contained by the actual Screen + * bounds an IllegalArgumentException will be thrown. + * + * @param x The screen X of the requested capture (must be >=0) + * @param y The screen Y of the requested capture (must be >=0) */ public int getPixelColor(int x, int y) { Application.checkEventThread(); +Screen s = Screen.getMainScreen(); +if (x < 0 || y < 0 || +x >= s.getWidth() || y > s.getHeight()) { + throw new IllegalArgumentException("Capture out of bounds"); +} return _getPixelColor(x
Re: Glass Robot and getSCreenCapture
It isn't public API now.. but keep in mind that making it public in some way has been requested for over two years. See RT-17571 Since those issues have made the internal Robot API known, no doubt people are using it. (We all know how well people listen to the warnings about using internal APIs .. *cough* sun.misc.Unsafe *cough*) Cheers, Scott On Wed, Mar 26, 2014 at 10:59 AM, Stephen F Northover wrote: > Hi David, > > Sorry to be getting to this so late. An uninitialized pixel is normally > considered to be black. If you throw an exception, clients will need to > either catch the exception or perform the same test that you are performing > before they call the "API". Since this is not pubic API, no client will be > affected so even if we make the change to throw the exception and then > decide not to do this later, we can change it. > > What is happening now? Who is being affected by this bug? Is it easy to > change the implementations to return black? This seems better to me than > throwing the exception. > > As I say, if we throw the exception, then we will only break ourselves, not > clients of FX API. Have we ensured that the exception will not break SQE > tests. > > Again, sorry to be getting to this so late and apologies if all of this has > been discussed in another thread that I missed, > Steve > > > On 2014-03-25 2:46 PM, David Hill wrote: >> >> On 3/21/14, Mar 21, 12:53 PM, David Hill wrote: >> >> Having heard a little feedback, here is my proposal in the form of a >> review request :-) >>> >>> >>> >>> My recommendation would be modify the JavaDoc contract to specify that >>> the x,y and x+width, y+height must be within the screen bounds, with an >>> IllegalArgumentException if out of bounds. This would be checked in Robot, >>> prior to calling the native impls. >>> >>> This code is "internal" API, so I expect the real impact would be on SQE. >> >> I really like the idea of adding a bounds restriction - so that the >> requested bounds must be within the Screen. >> It seems the simplest solution to my issue of handling the odd edge case >> of out of bound pixels, with the least likely impact. >> >> This means that existing code in the implementations are not affected. >> I suspect that there will we little if any impact on SQE tests, given that >> most of us would avoid asking for a screen capture with undefined pixels. I >> do expect that we will encounter a few exceptions to this when tests are run >> on smaller displays (like embedded). >> >> I also added bounds checking to Robot.getPixelColor() for consistency, and >> because Embedded passes this call through to common code for screen capture. >> >> I did a grep through the JavaFX code base, and don't see any JavaFX use >> cases. I expect any "golden image" test code could be affected. >> >> I separated out this internal API changes from my embedded changes so we >> have a clear and easy thing to review. >> >> Jira: https://javafx-jira.kenai.com/browse/RT-36382 >> >> Patch: is inline in the Jira, but also here: >> >> diff -r bb72bd2fa889 >> modules/graphics/src/main/java/com/sun/glass/ui/Robot.java >> --- a/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue Mar >> 25 14:21:26 2014 -0400 >> +++ b/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue Mar >> 25 14:41:37 2014 -0400 >> @@ -144,9 +144,20 @@ >> protected abstract int _getPixelColor(int x, int y); >> /** >> * Returns pixel color at specified screen coordinates in IntARGB >> format. >> + * >> + * If the requested pixel is not contained by the actual Screen >> + * bounds an IllegalArgumentException will be thrown. >> + * >> + * @param x The screen X of the requested capture (must be >=0) >> + * @param y The screen Y of the requested capture (must be >=0) >> */ >> public int getPixelColor(int x, int y) { >> Application.checkEventThread(); >> +Screen s = Screen.getMainScreen(); >> +if (x < 0 || y < 0 || >> +x >= s.getWidth() || y > s.getHeight()) { >> + throw new IllegalArgumentException("Capture out of bounds"); >> +} >> return _getPixelColor(x, y); >> } >> >> @@ -162,13 +173,27 @@ >> * will result in a Pixels object with dimensions (20x20). Calling >> code >> * should use the returned objects's getWidth() and getHeight() >> methods >> * to determine the image size. >> - * >> + * >> * If (@code isHiDPI) is {@code false}, the returned Pixels object is >> of >> * the requested size. Note that in this case the image may be scaled >> in >> * order to fit to the requested dimensions if running on a HiDPI >> display. >> + * >> + * If the requested capture bounds is not contained by the actual >> Screen >> + * bounds an IllegalArgumentException will be thrown. >> + * >> + * @param x The screen X of the requested capture (must be >=0) >> + * @param y The screen Y of the request
Re: Glass Robot and getSCreenCapture
Hi David, Sorry to be getting to this so late. An uninitialized pixel is normally considered to be black. If you throw an exception, clients will need to either catch the exception or perform the same test that you are performing before they call the "API". Since this is not pubic API, no client will be affected so even if we make the change to throw the exception and then decide not to do this later, we can change it. What is happening now? Who is being affected by this bug? Is it easy to change the implementations to return black? This seems better to me than throwing the exception. As I say, if we throw the exception, then we will only break ourselves, not clients of FX API. Have we ensured that the exception will not break SQE tests. Again, sorry to be getting to this so late and apologies if all of this has been discussed in another thread that I missed, Steve On 2014-03-25 2:46 PM, David Hill wrote: On 3/21/14, Mar 21, 12:53 PM, David Hill wrote: Having heard a little feedback, here is my proposal in the form of a review request :-) My recommendation would be modify the JavaDoc contract to specify that the x,y and x+width, y+height must be within the screen bounds, with an IllegalArgumentException if out of bounds. This would be checked in Robot, prior to calling the native impls. This code is "internal" API, so I expect the real impact would be on SQE. I really like the idea of adding a bounds restriction - so that the requested bounds must be within the Screen. It seems the simplest solution to my issue of handling the odd edge case of out of bound pixels, with the least likely impact. This means that existing code in the implementations are not affected. I suspect that there will we little if any impact on SQE tests, given that most of us would avoid asking for a screen capture with undefined pixels. I do expect that we will encounter a few exceptions to this when tests are run on smaller displays (like embedded). I also added bounds checking to Robot.getPixelColor() for consistency, and because Embedded passes this call through to common code for screen capture. I did a grep through the JavaFX code base, and don't see any JavaFX use cases. I expect any "golden image" test code could be affected. I separated out this internal API changes from my embedded changes so we have a clear and easy thing to review. Jira: https://javafx-jira.kenai.com/browse/RT-36382 Patch: is inline in the Jira, but also here: diff -r bb72bd2fa889 modules/graphics/src/main/java/com/sun/glass/ui/Robot.java --- a/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue Mar 25 14:21:26 2014 -0400 +++ b/modules/graphics/src/main/java/com/sun/glass/ui/Robot.java Tue Mar 25 14:41:37 2014 -0400 @@ -144,9 +144,20 @@ protected abstract int _getPixelColor(int x, int y); /** * Returns pixel color at specified screen coordinates in IntARGB format. + * + * If the requested pixel is not contained by the actual Screen + * bounds an IllegalArgumentException will be thrown. + * + * @param x The screen X of the requested capture (must be >=0) + * @param y The screen Y of the requested capture (must be >=0) */ public int getPixelColor(int x, int y) { Application.checkEventThread(); +Screen s = Screen.getMainScreen(); +if (x < 0 || y < 0 || +x >= s.getWidth() || y > s.getHeight()) { + throw new IllegalArgumentException("Capture out of bounds"); +} return _getPixelColor(x, y); } @@ -162,13 +173,27 @@ * will result in a Pixels object with dimensions (20x20). Calling code * should use the returned objects's getWidth() and getHeight() methods * to determine the image size. - * + * * If (@code isHiDPI) is {@code false}, the returned Pixels object is of * the requested size. Note that in this case the image may be scaled in * order to fit to the requested dimensions if running on a HiDPI display. + * + * If the requested capture bounds is not contained by the actual Screen + * bounds an IllegalArgumentException will be thrown. + * + * @param x The screen X of the requested capture (must be >=0) + * @param y The screen Y of the requested capture (must be >=0) + * @param width The of width the requested capture (must be >=1 and fit on the screen) + * @param height The of width the requested capture (must be >=1 and fit on the screen) + * @param isHiDPI return HiDPI if available */ public Pixels getScreenCapture(int x, int y, int width, int height, boolean isHiDPI) { Application.checkEventThread(); +Screen s = Screen.getMainScreen(); +if (x < 0 || y < 0 || +x + width > s.getWidth() || y + height > s.getHeight()) { + throw new IllegalArgumentException("Capture out of bounds"); +} return _getScreen
Monadic operations on ObservableValue
EasyBind [1] now adds monadic operations on ObservableValue: http://tomasmikula.github.io/blog/2014/03/26/monadic-operations-on-observablevalue.html [1] http://www.fxmisc.org/easybind/
Re: file attachment in Jira
I'm so sorry about this. This is a silly and draconian restriction that most open source projects do not face. I have pinged the Mordac-like elements that govern us once more, however, I have no direct control over the outcome. Steve On 2014-03-26 9:22 AM, Pedro Duque Vieira wrote: Hi, I know this has been discussed before, in one instance it was said file attachment in jira would not be possible due to internal possibles, later it was said this should be possible and would be dealt with. I think it would be important for this to be possible given the highly collaborative nature of the javafx project. This certainly is a hurdle to people who want to collaborate. IMHO attaching text/image files doesn't involve security risks. Thanks, best regards,
Re: Adding GStreamer plugins
I added a wiki link: https://wiki.openjdk.java.net/pages/viewpage.action?pageId=18808963 I welcome free will participants to go over the steps, upgrade GStreamer and eventually create a precise version of the guide. I will provide as much help as I can. K On 26.03.2014 16:43, Kirill Kirichenko wrote: Michael, On 26.03.2014 04:11, Michael Berry wrote: Kirill - I think I'll take your suggestion next and start looking at upgrading the existing native components to the latest version of GStreamer before I look at adding any more plugins, that would seem to make sense. Have you any pointers in how best to approach this? No pointer at all. I have it my my head. And it's about time to pass this experience before I forget it =) The thing is I did it once and it took me ~ 2 months. Here is the brief plan that you need to keep: [1] Start with the lower lever. It's glib. Linux doesn't need any glib update - we use the system glib. So glib update is necessary for Win/Mac. [1.1] Latest glib has plenty of dependencies on other 3-rd party libraries. But there is one that's mandatory - libFFI. Fortunately Oracle has approval to use libFFI in it's products. But this probably isn't necessary for OpenJFX. So at you first step you need to bring libFFI sources, place them in rt/modules/media/src/main/native/gstreamer/3rd_party/libffi and make sure it builds on Windows and Mac producing lib/a for static linking. You can probably make dll/dylib instead but I don't think it's necessary. [1.2] Take the latest glib 2.38 or even 2.40. Place them in rt/modules/media/src/main/native/gstreamer/3rd_party/glib Note that you don't need all sources/headers. But to remove precisely what's redundant you should first compile gstreamer/plugins. Here you make sure you can compile and build glib-lite.dll/libglib-lite.dylib Having done 1.2 you should be able to run media component with the new glib-lite.dll. If it runs then you're done with glib upgrade. It's important to apply fixes that we made in glib to the newer glib library. You can find them by grepping for GSTREAMER_LITE in sources/headers. [2] GStreamer update. [3] Oracle plugins compilation/update. This step will also be necessary because 0.10.35 API is different from 1.0. For Example GstBuffer that we extensively use has incompatible APIs. I won't get deeper into details for [2] and [3] now. Let's just handle [1] and then continue.
file attachment in Jira
Hi, I know this has been discussed before, in one instance it was said file attachment in jira would not be possible due to internal possibles, later it was said this should be possible and would be dealt with. I think it would be important for this to be possible given the highly collaborative nature of the javafx project. This certainly is a hurdle to people who want to collaborate. IMHO attaching text/image files doesn't involve security risks. Thanks, best regards, -- Pedro Duque Vieira
Re: Adding GStreamer plugins
https://javafx-jira.kenai.com/browse/RT-18009 This JIRA is covering adding support for more media formats. Since we are just talking about MKV, please open a separate JIRA to cover this work and attach the patch there. We can link to the new JIRA from RT-18009. Steve On 2014-03-25 7:47 PM, Jonathan Giles wrote: Typically in this case you would email the patch to the assigned developer, but it appears RT-18009 is unassigned at present. I think that is the first hurdle that needs to be resolved, but if you email me your patch I will attach it to the jira issue so that it is at least available for others. Thanks! -- Jonathan On 26/03/2014 12:39 p.m., Michael Berry wrote: Hi all, Turns out it was a stupid mistake on my part causing the last few linking errors, I hadn't added one of the C files I needed to the makefile (well I had, but I'd then reverted it again without realising!) Thanks to all for the prods and advice. Once I'd done that, the build went through without a hitch - and then it was just a case of making the relevant additions to the native code to register the MKV type and create a pipeline from it using the plugin. This wasn't hard to work out at all; I've since tried several test files in MKV format (with AAC audio) all of which play in MediaPlayer without a hitch! I mainly wanted to do this as a personal exercise, though I'd imagine this is a useful piece of functionality for many others also - so should I submit a patch of the changes, or is this unlikely to be accepted? (Again, sorry for the perhaps obvious question, I'm rather new to this.) I've had a look at the code review process and it seems to suggest adding a patch to the relevant JIRA issue for those who don't have commit access (in this case 18009), but I don't seem to have permission to do that either? Thanks, Michael On 25 March 2014 17:01, Stephen F Northover wrote: On 2014-03-25 7:00 AM, Kirill Kirichenko wrote: Hi Michael. See my comments inline. On 24.03.2014 04:31, Michael Berry wrote: I'm now a bit further along with this, though struggling to get the matroska plugin to compile (getting a bunch of unresolved external symbol errors for functions it uses in glib - not entirely sure why at the moment, as I said C is not my strong point.) I've also noticed the plugins currently bundled have quite a few changes to the gstreamer version, and I can't quite work out the logic behind why things have been changed the way they have - so even after the compilation issue is resolved I'm now less confident that it will just drop in and work! Again, if someone knowledgeable in this area that's lurking in the shadows could shed any light on any of this, it would be hugely appreciated. We did some changes in existing GStreamer code because it had errors and because we needed to expand some functionality. The changes are not very extensive. However, putting the current problems aside for a bit, the snags I hit up until this point could I think be relatively easily addressed in the documentation. With that in mind could I suggest a few additional points for the wiki? These may be obvious to the majority reading here, but as someone completely new to building JFX they had me stumped for a while! I can give you some directions. There is no wiki. I'd appreciate if you created one. - It turns out that the Gstreamer stuff doesn't compile at all by default, which is why I wasn't seeing any changes on the native level. To ensure GStreamer is actually compiled, you need to copy the gradle.properties.template file to gradle.properties, and uncomment the "#COMPILE_MEDIA = true" line. (A similar scenario would appear to exist for any webkit alterations as per the line above.) You don't need to comment/uncomment anything. Just add -PCOMPILE_MEDIA=true to the gradle command line. You can however change the properties file too. - As well as the requirements listed, I needed the Windows SDK ( http://www.microsoft.com/en-gb/download/details.aspx?id=8279) installed for GStreamer to compile successfully under Windows (7) - without it cygpath just threw a rather confusing error. You need windows 7.1a SDK and speaking precisely you need only samples from it because samples have BaseClasses at Samples/multimedia/directshow/baseclasses. BaseClasses are used for Oracle direchshowwrapper plugin. - The developer workflow page ( https://wiki.openjdk.java.net/display/OpenJFX/Developer+Work+Flow) refers to "-Djavafx.ext.dirs" - I think this should be "-Djava.ext.dirs" instead? What are you gonna use it for ? I updated the page and got rid of the BINARY_STUB reference that is no longer needed. I'm happy to make the above changes myself but unsure of if / where you can sign up for an account, so I'm just throwing them here for now - if anyone could point me to the right place then that'd be great! Steve. Can you give an advice ? If you are looking to c
Re: Adding GStreamer plugins
Michael, On 26.03.2014 04:11, Michael Berry wrote: Kirill - I think I'll take your suggestion next and start looking at upgrading the existing native components to the latest version of GStreamer before I look at adding any more plugins, that would seem to make sense. Have you any pointers in how best to approach this? No pointer at all. I have it my my head. And it's about time to pass this experience before I forget it =) The thing is I did it once and it took me ~ 2 months. Here is the brief plan that you need to keep: [1] Start with the lower lever. It's glib. Linux doesn't need any glib update - we use the system glib. So glib update is necessary for Win/Mac. [1.1] Latest glib has plenty of dependencies on other 3-rd party libraries. But there is one that's mandatory - libFFI. Fortunately Oracle has approval to use libFFI in it's products. But this probably isn't necessary for OpenJFX. So at you first step you need to bring libFFI sources, place them in rt/modules/media/src/main/native/gstreamer/3rd_party/libffi and make sure it builds on Windows and Mac producing lib/a for static linking. You can probably make dll/dylib instead but I don't think it's necessary. [1.2] Take the latest glib 2.38 or even 2.40. Place them in rt/modules/media/src/main/native/gstreamer/3rd_party/glib Note that you don't need all sources/headers. But to remove precisely what's redundant you should first compile gstreamer/plugins. Here you make sure you can compile and build glib-lite.dll/libglib-lite.dylib Having done 1.2 you should be able to run media component with the new glib-lite.dll. If it runs then you're done with glib upgrade. It's important to apply fixes that we made in glib to the newer glib library. You can find them by grepping for GSTREAMER_LITE in sources/headers. [2] GStreamer update. [3] Oracle plugins compilation/update. This step will also be necessary because 0.10.35 API is different from 1.0. For Example GstBuffer that we extensively use has incompatible APIs. I won't get deeper into details for [2] and [3] now. Let's just handle [1] and then continue.
Re: Adding GStreamer plugins
There is another issue with new plugins. We have thoroughly tested with a closed source internal test suite on all platforms every plugin that we already have. If we add something new this may open a way for crashes, security breaches. So if you add something new and want this to be included in Oracle JavaFX (JDK) this needs to be tested well. As for OpenJFX - it's a good starting point. On 26.03.2014 12:58, Michael Berry wrote: Hi David, Sure - I'm aware there's legal issues surrounding many of the formats, though one of the reasons I picked MKV to start with was because it's an open container format. I'm certainly not aware of any restrictions surrounding it (though please correct me if I'm wrong on that front!) A switch certainly sounds like a good idea though, especially for formats with more restrictions surrounding their use; I'll bear that in mind if I add any additional plugins. Thanks, Michael On 26 March 2014 02:03, David DeHaven wrote: I mainly wanted to do this as a personal exercise, though I'd imagine this is a useful piece of functionality for many others also - so should I submit a patch of the changes, or is this unlikely to be accepted? (Again, sorry for the perhaps obvious question, I'm rather new to this.) I've had a look at the code review process and it seems to suggest adding a patch to the relevant JIRA issue for those who don't have commit access (in this case 18009), but I don't seem to have permission to do that either? It sounds like there are perhaps two different ways to move forward here, the first is to take a submission in the form of WIKI writeup that we can post so that anybody else who wants to try extending the media files can follow the path you took. The other is to take a patch for the given JIRA issue. Sadly, the JIRA server doesn't allow just anybody to supply patches, so you will have to email to Steve or Kevin and they can attach it to the issue for you. Be careful how these are implemented. We cannot just enable formats in GStreamer, there are licensing and legal issues involved. I think the Matroska licenses are fairly benign, but it still requires some amount of process. What I would recommend is adding a switch to optionally enable additional formats, so those building GStreamer or OpenJFX themselves can turn whatever they want on or off, and they are on their own for dealing with legal issues. That being said, we actually tested with MKV during development and it was pretty solid :) -DrD-
hg: openjfx/8u-dev/rt: RT-35263: Win: Crashing VM in a JavaFX 3D app reading and writing STL files
Changeset: 89ca2db487e3 Author:Anthony Petrov Date: 2014-03-26 15:13 +0400 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/89ca2db487e3 RT-35263: Win: Crashing VM in a JavaFX 3D app reading and writing STL files Summary: Check if the window is still alive after handling a synthetic mouse ENTER event Reviewed-by: snorthov, fheidric ! modules/graphics/src/main/native-glass/win/ViewContainer.cpp
Re: Adding GStreamer plugins
Hi David, Sure - I'm aware there's legal issues surrounding many of the formats, though one of the reasons I picked MKV to start with was because it's an open container format. I'm certainly not aware of any restrictions surrounding it (though please correct me if I'm wrong on that front!) A switch certainly sounds like a good idea though, especially for formats with more restrictions surrounding their use; I'll bear that in mind if I add any additional plugins. Thanks, Michael On 26 March 2014 02:03, David DeHaven wrote: > > >> I mainly wanted to do this as a personal exercise, though I'd imagine > this > >> is a useful piece of functionality for many others also - so should I > >> submit a patch of the changes, or is this unlikely to be accepted? > (Again, > >> sorry for the perhaps obvious question, I'm rather new to this.) I've > had a > >> look at the code review process and it seems to suggest adding a patch > to > >> the relevant JIRA issue for those who don't have commit access (in this > >> case 18009), but I don't seem to have permission to do that either? > > > > It sounds like there are perhaps two different ways to move forward > here, the first is to take a submission in the form of WIKI writeup that we > can post so that anybody else who wants to try extending the media files > can follow the path you took. The other is to take a patch for the given > JIRA issue. Sadly, the JIRA server doesn't allow just anybody to supply > patches, so you will have to email to Steve or Kevin and they can attach it > to the issue for you. > > Be careful how these are implemented. We cannot just enable formats in > GStreamer, there are licensing and legal issues involved. I think the > Matroska licenses are fairly benign, but it still requires some amount of > process. > > What I would recommend is adding a switch to optionally enable additional > formats, so those building GStreamer or OpenJFX themselves can turn > whatever they want on or off, and they are on their own for dealing with > legal issues. > > That being said, we actually tested with MKV during development and it was > pretty solid :) > > -DrD- > >
hg: openjfx/8u-dev/rt: RT-36371 GridPane ColumnConstraints Java 7 and 8 broken compatibility
Changeset: 14580ce9a75e Author:Martin Sladecek Date: 2014-03-26 09:39 +0100 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/14580ce9a75e RT-36371 GridPane ColumnConstraints Java 7 and 8 broken compatibility ! modules/graphics/src/main/java/javafx/scene/layout/GridPane.java ! modules/graphics/src/test/java/javafx/scene/layout/GridPaneTest.java
Re: Glass Robot and getSCreenCapture
It should be throwing an exception or returning null, returning a real value is a code smell. Sent from my iPhone > On 23 Mar 2014, at 07:55, Daniel Blaukopf wrote: > > We should be consistent about what we return, although I agree that that the > actual value doesn’t seem to matter. 0 for imaginary pixels seems reasonable. > >> On Mar 21, 2014, at 7:05 PM, Anthony Petrov >> wrote: >> >> Hi David, >> >> I don't think we're making any assumptions. We feed the coordinates to a >> native API and rely on the OS to do the right thing. >> >> In other words, our assumption is that if the box lays (partially or fully) >> outside of the screen area, then the behavior is undefined. Note that the >> Screen API in Glass allows its clients to check what coordinates are valid >> (i.e. belong to a real screen). >> >> So whatever your return for pixels outside of screen bounds should be fine. >> 0x0 or 0xff00 - both look good. However, I agree that a stricter >> specification and a check might be the best solution. >> >> -- >> best regards, >> Anthony >> >>> On 3/21/2014 8:53 PM, David Hill wrote: >>> >>> I have been working on a problem with Robot.getScreenCapture() on a 565 >>> ARM device, and while doing so, encountered a couple of questions which >>> I will bring up: >>> >>> Pixxls getScreenCapture(int x, int y, int width, int height, boolean >>> isHiDPI) >>> >>> I don't seem any real documentation that says how x,y + width,height >>> should be treated when compared to the reality of the Screen. >>> What values of x,y + width,height are reasonable ? I can picture any >>> number of scenarios with them that would result in a box that does not >>> fit within the Screen dimensions. The only implementing code I have seen >>> checks to that the width & height are >= 1. Can I/Should I handle -x >>> values ? What if the requested bounds exceed the screen ? >>> >>> If we are making assumptions that the requested box is inside the >>> screen then why don't we document that fact and add a check in the >>> Robot class (instead of relying on the native impls). >>> >>> If we are assuming the requested box does not have to lie within the >>> screen bounds - what should the returned values be for the pixels >>> outside the screen ? Pixel Black ? (Currently I think Lens would return >>> 0x instead of 0xff00) >>> >>> My recommendation would be modify the JavaDoc contract to specify that >>> the x,y and x+width, y+height must be within the screen bounds, with an >>> IllegalArgumentException if out of bounds. This would be checked in >>> Robot, prior to calling the native impls. >>> >>> This code is "internal" API, so I expect the real impact would be on SQE. >