Re: [OpenJDK 2D-Dev] [9] Review request for 8069361 SunGraphics2D.getDefaultTransform() does not include scale factor
The math looks fine to me. We'll need to coordinate this with changes for Windows HiDPI as well... ...jim On 4/20/15 4:48 AM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8069361/webrev.01/ - CGraphicsConfig.getDefaultTransform() is updated to use AffineTransform.getScaleInstance() - SG2D.getDefaultTransform() is updated to not check GraphicsConfiguration.getDefaultTransform() on null - the test is updated to compare SG2D transform with GraphicsConfiguration transform on all graphics configurations Thanks, Alexandr. On 4/17/2015 4:28 PM, Sergey Bylokhov wrote: Hi, Alexander. I assume that the code in SG2D.getTransform/setTransform is the same as was before the fix of 8000629. Code in SG2D.getDefaultTransform can be simplified, id do not think that GraphicsConfiguration.getDefaultTransform. and SG2D.getDeviceConfiguration can return null for NullSurfaceData only. The test should check that default transform of graphics configuration(all screens should be checked) is the same as a transform of the Graphics. The method CGraphicsConfig.getDefaultTransform() can use getScaleInstance but it is up to you. On 14.04.15 17:51, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8069361 webrev: http://cr.openjdk.java.net/~alexsch/8069361/webrev.00 SunGraphics2D.getDefaultTransform() now returns transform which includes GraphicsConfiguration default transform. This can break a compatibility for applications which assumes that default Graphics2D transform is always identity and restore it as sg2d.setTransform(new AffineTransform()). However, this is not now true for HiDPI displays. Thanks, Alexandr.
[9] Review Request: JDK- 8078082 Fix for [TEST_BUG] java/awt/SplashScreen/MultiResolutionSplash/MultiResolutionSplashTest.java fails
Hello, Please review a fix for issue : 8078082 [TEST_BUG] Test java/awt/SplashScreen/MultiResolutionSplash/MultiResolutionSplashTest.java fails Test bug fix. https://bugs.openjdk.java.net/browse/JDK-8078082 The webrev is : http://cr.openjdk.java.net/~pchopra/8078082/webrev.00/ Thanks , Pooja
Re: [9] Review request for 8044444 The output's 'Page-n' footer does not show completely.
Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/804/webrev.04
On 4/17/2015 2:03 AM, Phil Race wrote:
It looks a bit odd that the new methods in RPJ.java don't use the
PageFormat parameter since its needed only in the subclass over-ride ..
But at least the shared code logic remains the same.
Did you look into this earlier comment :-
>I am not entirely sure that in the case you use printDialog() with
no-args
>and print() with no-args that we really should be in this
attributesToPageFormat()
>method at all.
The RasterPrinterJob.attributes were null in case printDialog()
with no-args was used before the regression so attributeToPageFormat()
was not called.
I added additional attributes.isEmpty() check to the
getPageFormatFromAttributes() to preserve this behavior.
The test still has this problem I mentioned :-
In the test 'custom' case I see you use printDialog(attributeset) but
then print()
The normal pattern is to use it consistently so that the changes made
by the
user in the attributeset are propagated.
The test is updated.
Thanks,
Alexandr.
150
151 boolean printAccepted = job.printDialog(printAttributes);
152 if (printAccepted) {
153 try {
154 job.print();
-phil.
On 3/12/15 8:47 AM, Alexander Scherbatiy wrote:
Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/804/webrev.03
On 12/4/2014 4:17 AM, Phil Race wrote:
This looks like it might help in so far as it implies that the
bug was an incorrect guess at what should be the imageable area
But I have a lot of uncertainties that I need to investigate.
Eg when I try your test case with 8u20 I don't see a vertical margin
of anywhere near 1 inch so if we are getting margins from the code
you over-rode, why is that ? Is JTable deliberately drawing outside
the imageable area for its header and footer ?
I tried it with earlier JDK and I see the same behavior. At least
it is not a regression.
Can we just return with defaultPage ? Not if there are updated margins
from somewhere else.
Margins can be set by a user.
I am not entirely sure that in the case you use printDialog() with
no-args
and print() with no-args that we really should be in this
attributesToPageFormat()
method at all. Perhaps we can fix it up when we are here but can we
avoid
this ?
printLoop method from CPrinterJob.m file has the special
workaround for JTable.print that leads the attributeToPageFormat()
method is called:
// JTable.print attributes are ignored
jobject pageable = JNFCallObjectMethod(env, jthis,
jm_getPageable); // AWT_THREADING Safe (!appKit)
javaPrinterJobToNSPrintInfo(env, jthis, pageable, printInfo);
Regardless of that there are couple of things that look like bugs in
the code :-
The PageFormat imageable area takes into account the rotation of the
page,
the Paper does not. So when you set it like this :-
747 paper.setImageableArea(page.getImageableX(),
page.getImageableY(),
748 page.getImageableWidth(),
page.getImageableHeight());
749 }
.. you are ignoring the potential for LANDSCAPE - or REVERSE_LANDSCAPE.
I updated it to use the paper imageable area.
And what if the default page is based on some large size like A3
and then the application has specified a media of A5, but no media
printable area ?
It appears the code will then try to set the paper's imageable area
much larger than the entire paper.
Should you not in fact limit it to the size of the paper ?
Or arguably limit it to the hardware limited imageable area ?
It is how it worked before the regression. It can be considered
as a separated issue.
Thanks,
Alexandr.
In the test 'custom' case I see you use printDialog(attributeset)
but then print()
The normal pattern is to use it consistently so that the changes
made by the
user in the attributeset are propagated.
On the mac the native 'print' dialog doesn't - so far as I can see -
allow you
to change the paper size and layout. This is a bit different than
other platforms.
I guess your bug manifests in the case where this is defaults so it
probably
doesn't matter.
-phil.
On 12/03/2014 06:08 AM, Alexander Scherbatiy wrote:
On 12/1/2014 8:28 PM, Phil Race wrote:
Hmm .. it looks as if this breaks the case when the Swing dialog
is used, doesn't it ?
I think this update needs to be accompanied by regression tests
that show that
this kind of page set up using native & swing dialogs both work.
We can't easily use the JCK tests for this.
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/804/webrev.02
- The native imageable area is set to the page if it is not
defined in the set of attributes for the printer job.
- The manual test that checks printing with/without print dialog
and with/without media printable area properties is added.
[9] Review Request: 6829245 Reg test: java/awt/Component/isLightweightCrash/StubPeerCrash.java fails
Hello. Please review a small fix for jdk 9. The test StubPeerCrash.java was removed for two reasons: - Another test for related bug was implemented(see comments in the bug) - After the fix for JDK-8074028 the AHeavyweightComponent.getPeer() method is ignored in the jdk. Bug: https://bugs.openjdk.java.net/browse/JDK-6829245 Webrev can be found at: http://cr.openjdk.java.net/~serb/6829245/webrev.00 -- Best regards, Sergey.
Re: [9] Review request for 8069361 SunGraphics2D.getDefaultTransform() does not include scale factor
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8069361/webrev.01/ - CGraphicsConfig.getDefaultTransform() is updated to use AffineTransform.getScaleInstance() - SG2D.getDefaultTransform() is updated to not check GraphicsConfiguration.getDefaultTransform() on null - the test is updated to compare SG2D transform with GraphicsConfiguration transform on all graphics configurations Thanks, Alexandr. On 4/17/2015 4:28 PM, Sergey Bylokhov wrote: Hi, Alexander. I assume that the code in SG2D.getTransform/setTransform is the same as was before the fix of 8000629. Code in SG2D.getDefaultTransform can be simplified, id do not think that GraphicsConfiguration.getDefaultTransform. and SG2D.getDeviceConfiguration can return null for NullSurfaceData only. The test should check that default transform of graphics configuration(all screens should be checked) is the same as a transform of the Graphics. The method CGraphicsConfig.getDefaultTransform() can use getScaleInstance but it is up to you. On 14.04.15 17:51, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8069361 webrev: http://cr.openjdk.java.net/~alexsch/8069361/webrev.00 SunGraphics2D.getDefaultTransform() now returns transform which includes GraphicsConfiguration default transform. This can break a compatibility for applications which assumes that default Graphics2D transform is always identity and restore it as sg2d.setTransform(new AffineTransform()). However, this is not now true for HiDPI displays. Thanks, Alexandr.
Re: RFR: JDK-8074859 Turn on warnings as error
Looks good to me. /Erik On 2015-04-17 14:52, Magnus Ihse Bursie wrote: With JDK-8074096, the number of warnings in the product was reduced to a minimum. This enables the next step, which is turning on the respective compiler flags that turns warnings into errors. In the long run, this is the only way to keep the warnings from creeping back. Even with JDK-8074096, the product does not build 100% warning free. This is due to some warnings that cannot be disabled, or (in one case) where C and C++ code is mixed, and warnings for both languages cannot be used. A system similar to the one introduced in JDK-8074096 is therefore needed, in which individual libraries can be exempted from this flag, until such warnings are fixed. A library can thus disable warnings as errors with WARNINGS_AS_ERRORS := false, or (better) use a toolchain-specific version, e.g WARNINGS_AS_ERRORS_gcc := false. This is intended as a temporary measure, though. The long-term solution is reasonably to fix the warnings and remove that argument. Also, different versions of compilers can generate a different set of warnings. It is therefore necessary to be able to turn off this globally. Therefor a new flag for configure is introduced: --disable-warnings-as-errors. While the code compiles without errors on the build systems used internally at Oracle, this might not be the case on other combinations of operating system versions and toolchain versions. To facilitate for unexpecting developers, a help message is added if the build fails, that suggests using --disable-warnings-as-errors. This solution was chosen as a compromise between the "hard core" solution of turning on warnings as errors by default for anyone, and the cowar... erm, conservative solution of checking if the compiler versions exactly match what's used inside Oracle (and therefore regularly tested), and only turn it on in that case. Similarly to JDK-8074096, I intend to file follow-up bugs for each individual library that got a WARNINGS_AS_ERRORS_* := false. Bug: https://bugs.openjdk.java.net/browse/JDK-8074859 WebRev for top: http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-top/webrev.01 WebRev for jdk: http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-jdk/webrev.01 Some comments: * I needed to add a few more DISABLED_WARNINGS. For windows, this is most likely due to the recent compiler change. For other libraries, I'm not sure, but it might well be the result of recent changes that has introduced new warnings. If so, it highlights the need of this patch to keep the build warning free. * For a few libraries and toolchains, there is *both* WARNINGS_AS_ERRORS := false and a DISABLED_WARNINGS list. This is the case if not all warnings are possible to disable. * I have removed some incorrect uses of SHARED_LIBRARY_FLAGS. This is included in our JDK LDFLAGS, so it should not be set separately, and definitely not as CFLAGS. (This caused compiler warnings, which now turned into errors.) However, a more suitable long-term solution is probably to move the knowledge of how to create shared libraries more specifically into SetupNativeCompilation, and not set it as part of the JDK flags. /Magnus
