Re: [OpenJDK 2D-Dev] [13] Review Request: 8224171 The cleanup multi-font related code in the XFontPeer

[Semyon Sadetsky]

I don't know how timely it is to cleanup this code. Anyway, you've 
removed global awtJNI_GetFontData() function but left its external 
declaration in open/src/java.desktop/unix/native/common/awt/awt_p.h:


http://hg.openjdk.java.net/jdk/client/file/f680bedc0dcb/src/java.desktop/unix/native/common/awt/awt_p.h#l122 



Since native code was modified can you provide the prove that the change 
doesn't brake the build?


--Semyon

On 5/18/19 3:10 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for JDK 13.

Bug: https://bugs.openjdk.java.net/browse/JDK-8224171
Fix: http://cr.openjdk.java.net/~serb/8224171/webrev.00

This change is the part of my effort to clean up the native 
initialization code in awt/2d. Initially, I have dropped the native 
code inside initIds() which was unused(JDK-8223766[1]). Now I tried to 
check is the code inside initIDs() is used for some meaningful purpose.


I found that XFontPeer.initIds() uses XFontPeer.xfsname field which is 
always null, so in this fix, I have dropped the field itself, the 
initIds(), the methods which depend on this field, and methods which 
calls methods which depends on this field, etc. Next step will be 
clean up the native code for the Fonts itself, there is some unused 
code as well.


Note that this field was unused since 2009[2]

[1] https://bugs.openjdk.java.net/browse/JDK-8223766
[2] http://hg.openjdk.java.net/jdk/client/rev/a79da6a9d184

Re: [OpenJDK 2D-Dev] RFR: 8217731: Font rendering and glyph spacing changed from jdk-8 to jdk-11

[semyon . sadetsky]

On 6/6/19 9:12 AM, Phil Race wrote:




On 6/6/19 9:11 AM, semyon.sadet...@oracle.com wrote:

On 6/5/19 6:43 PM, Philip Race wrote:




On 6/5/19, 4:18 PM, Semyon Sadetsky wrote:

On 6/5/19 2:11 PM, Phil Race wrote:


It *can* make a difference and in fact we have a regression test
that now passes with this fix which tests different rendering modes.

Which test is it? Why  you didn't mark it with the bug id?


See https://bugs.openjdk.java.net/browse/JDK-8199529

I only located this bug and verified this "resolves" it after 
sending out the review
but it "resolves" it due to luck as much as anything definite, so I 
don't think it is

required to link this as "solving" that.

Nice that you made this discovery. Though it was more or less obvious.
So, now you can remove jtreg-hard label from the bug and provide the 
reg-test.


Again, no.

Why? The test is not hard.



However that is not a direct test for this potential difference.
You cannot say that this change *must* make a difference, it
just does. Sometimes.


That's what we need to avoid regression again when fonts are 
updated. Font appearance directly affects user experience. 
Fortunately this happens not so often but we definitely need a test 
that will indicate such changes before the bug is reported 
externally like it recently happened. I thin everyone agrees that 
we should not repeat this omission once again.


You misunderstand. There was no regression. 

Then why is the bug marked as regression?


Not by me. But it is subjective.

Then you need to remove the label and update evaluation accordingly.




There was a change in behaviour
which is completely allowable, and can happen all the time and is 
sensitive to
so many things. So there was no omission. 

Ok, then why do we need this fix?


To try to improve compatibility of rendering in 13 with what it was 
like in 8.

No guarantees but people have reported they prefer it.

Can you argument your position?
For me it is a bug.



Nothing can be tested and asserted
to be right or wrong. And the algorithms used are outside of our 
control.
Was it external change in Windows OS that caused the issue? If so, 
the bug was incorrectly evaluated. Can you update JBS with the 
correct one.


No, it was the removal of T2K and the switch to freetype.

I'm confused again, why it is not our regression?

--Semyon

-phil.

But there is still value in the change to see if more people are 
happy with the

alternative rendering.

--Semyon


-phil





--Semyon



-phil.

On 6/5/19 1:40 PM, semyon.sadet...@oracle.com wrote:

Can you clarify does the change affects font metrics?

I see that it is a sub-pixel difference for each single glyph but 
if a long line of text can accumulate a notable difference the 
reg test can be provided.


--Semyon

On 6/5/19 11:43 AM, Phil Race wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8217731
webrev: http://cr.openjdk.java.net/~prr/8217731/

This is intended to "help" but cannot completely cure, with
some of the rendering differences in JDK11 vs JDK 8.
In a typical Swing app on Windows using LCD rendering
it manifests as subtle adjustments in the spacing between glyphs.
There isn't an easy regression test for this, and it is subjective
as to how bad it was before and how much this improves it,
even if you were to accept that 8 is "better" .. and not just 
different ..


-phil.

Re: [OpenJDK 2D-Dev] RFR: 8217731: Font rendering and glyph spacing changed from jdk-8 to jdk-11

[semyon . sadetsky]

On 6/5/19 6:43 PM, Philip Race wrote:




On 6/5/19, 4:18 PM, Semyon Sadetsky wrote:

On 6/5/19 2:11 PM, Phil Race wrote:


It *can* make a difference and in fact we have a regression test
that now passes with this fix which tests different rendering modes.

Which test is it? Why  you didn't mark it with the bug id?


See https://bugs.openjdk.java.net/browse/JDK-8199529

I only located this bug and verified this "resolves" it after sending 
out the review
but it "resolves" it due to luck as much as anything definite, so I 
don't think it is

required to link this as "solving" that.

Nice that you made this discovery. Though it was more or less obvious.
So, now you can remove jtreg-hard label from the bug and provide the 
reg-test.



However that is not a direct test for this potential difference.
You cannot say that this change *must* make a difference, it
just does. Sometimes.


That's what we need to avoid regression again when fonts are updated. 
Font appearance directly affects user experience. Fortunately this 
happens not so often but we definitely need a test that will indicate 
such changes before the bug is reported externally like it recently 
happened. I thin everyone agrees that we should not repeat this 
omission once again.


You misunderstand. There was no regression. 

Then why is the bug marked as regression?

There was a change in behaviour
which is completely allowable, and can happen all the time and is 
sensitive to
so many things. So there was no omission. 

Ok, then why do we need this fix?

Nothing can be tested and asserted
to be right or wrong. And the algorithms used are outside of our control.
Was it external change in Windows OS that caused the issue? If so, the 
bug was incorrectly evaluated. Can you update JBS with the correct one.
But there is still value in the change to see if more people are happy 
with the

alternative rendering.

--Semyon


-phil





--Semyon



-phil.

On 6/5/19 1:40 PM, semyon.sadet...@oracle.com wrote:

Can you clarify does the change affects font metrics?

I see that it is a sub-pixel difference for each single glyph but 
if a long line of text can accumulate a notable difference the reg 
test can be provided.


--Semyon

On 6/5/19 11:43 AM, Phil Race wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8217731
webrev: http://cr.openjdk.java.net/~prr/8217731/

This is intended to "help" but cannot completely cure, with
some of the rendering differences in JDK11 vs JDK 8.
In a typical Swing app on Windows using LCD rendering
it manifests as subtle adjustments in the spacing between glyphs.
There isn't an easy regression test for this, and it is subjective
as to how bad it was before and how much this improves it,
even if you were to accept that 8 is "better" .. and not just 
different ..


-phil.

Re: [OpenJDK 2D-Dev] RFR: 8217731: Font rendering and glyph spacing changed from jdk-8 to jdk-11

[Semyon Sadetsky]

On 6/5/19 2:11 PM, Phil Race wrote:


It *can* make a difference and in fact we have a regression test
that now passes with this fix which tests different rendering modes.

Which test is it? Why  you didn't mark it with the bug id?

However that is not a direct test for this potential difference.
You cannot say that this change *must* make a difference, it
just does. Sometimes.


That's what we need to avoid regression again when fonts are updated. 
Font appearance directly affects user experience. Fortunately this 
happens not so often but we definitely need a test that will indicate 
such changes before the bug is reported externally like it recently 
happened. I thin everyone agrees that we should not repeat this omission 
once again.


--Semyon



-phil.

On 6/5/19 1:40 PM, semyon.sadet...@oracle.com wrote:

Can you clarify does the change affects font metrics?

I see that it is a sub-pixel difference for each single glyph but if 
a long line of text can accumulate a notable difference the reg test 
can be provided.


--Semyon

On 6/5/19 11:43 AM, Phil Race wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8217731
webrev: http://cr.openjdk.java.net/~prr/8217731/

This is intended to "help" but cannot completely cure, with
some of the rendering differences in JDK11 vs JDK 8.
In a typical Swing app on Windows using LCD rendering
it manifests as subtle adjustments in the spacing between glyphs.
There isn't an easy regression test for this, and it is subjective
as to how bad it was before and how much this improves it,
even if you were to accept that 8 is "better" .. and not just 
different ..


-phil.

Re: [OpenJDK 2D-Dev] RFR: 8217731: Font rendering and glyph spacing changed from jdk-8 to jdk-11

[semyon . sadetsky]

Can you clarify does the change affects font metrics?

I see that it is a sub-pixel difference for each single glyph but if a 
long line of text can accumulate a notable difference the reg test can 
be provided.


--Semyon

On 6/5/19 11:43 AM, Phil Race wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8217731
webrev: http://cr.openjdk.java.net/~prr/8217731/

This is intended to "help" but cannot completely cure, with
some of the rendering differences in JDK11 vs JDK 8.
In a typical Swing app on Windows using LCD rendering
it manifests as subtle adjustments in the spacing between glyphs.
There isn't an easy regression test for this, and it is subjective
as to how bad it was before and how much this improves it,
even if you were to accept that 8 is "better" .. and not just 
different ..


-phil.

Re: [OpenJDK 2D-Dev] [11] Review Request: 8198406 Test TestAATMorxFont is unstable

[Semyon Sadetsky]

On 03/08/2018 02:24 PM, Sergey Bylokhov wrote:


On 08/03/2018 14:11, Semyon Sadetsky wrote:
That's doesn't make any sense. The w/o jtreg it is run manually and 
it is always known on what is the platform. There may be a need to 
run the test on another platform but this redundant check makes it 
not possible.


It was intentional decision, if this test will be run on other 
platform it will pass because it is targeted to macOS.

Then for what you have added platform filter in the jtreg header?


It will save the compilation and startup time.
It is neglectable since the test is very simple. Did you run test on 
other platform without this fake passage logic? There is a good chance 
that the test pases on all other platforms without any filters.

Re: [OpenJDK 2D-Dev] [11] Review Request: 8198406 Test TestAATMorxFont is unstable

[Semyon Sadetsky]

On 03/08/2018 12:47 PM, Sergey Bylokhov wrote:


On 08/03/2018 12:38, Semyon Sadetsky wrote:
This check was added for the case when the test will be run in 
standalone w/o jtreg
That's doesn't make any sense. The w/o jtreg it is run manually and 
it is always known on what is the platform. There may be a need to 
run the test on another platform but this redundant check makes it 
not possible.


It was intentional decision, if this test will be run on other 
platform it will pass because it is targeted to macOS.

Then for what you have added platform filter in the jtreg header?






--Semyon


On 03/07/2018 07:32 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk11.

Bug: https://bugs.openjdk.java.net/browse/JDK-8198406
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8198406/webrev.00


Initially, this test was marked as unstable, because sometimes it 
passed(when invokeLater did not start till the end of the main 
method) or failed when the code in invokeLater thrown an 
HeadlessException.


In the fix the test is reworked to use BufferedImage instead of 
Frame, so it will work in headful and headless environment.


Some additional info:
http://mail.openjdk.java.net/pipermail/2d-dev/2018-February/008947.html 

Re: [OpenJDK 2D-Dev] [11] Review Request: 8198406 Test TestAATMorxFont is unstable

[Semyon Sadetsky]

On 03/08/2018 12:03 PM, Sergey Bylokhov wrote:


On 08/03/2018 08:30, Semyon Sadetsky wrote:
Please remove redundant platform detection code since this is done in 
the jtreg header now.


This check was added for the case when the test will be run in 
standalone w/o jtreg
That's doesn't make any sense. The w/o jtreg it is run manually and it 
is always known on what is the platform. There may be a need to run the 
test on another platform but this redundant check makes it not possible.




--Semyon


On 03/07/2018 07:32 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk11.

Bug: https://bugs.openjdk.java.net/browse/JDK-8198406
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8198406/webrev.00


Initially, this test was marked as unstable, because sometimes it 
passed(when invokeLater did not start till the end of the main 
method) or failed when the code in invokeLater thrown an 
HeadlessException.


In the fix the test is reworked to use BufferedImage instead of 
Frame, so it will work in headful and headless environment.


Some additional info:
http://mail.openjdk.java.net/pipermail/2d-dev/2018-February/008947.html

Re: [OpenJDK 2D-Dev] [11] Review Request: 8198406 Test TestAATMorxFont is unstable

[Semyon Sadetsky]

Hi Sergey,

Please remove redundant platform detection code since this is done in 
the jtreg header now.


--Semyon


On 03/07/2018 07:32 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk11.

Bug: https://bugs.openjdk.java.net/browse/JDK-8198406
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8198406/webrev.00


Initially, this test was marked as unstable, because sometimes it 
passed(when invokeLater did not start till the end of the main method) 
or failed when the code in invokeLater thrown an HeadlessException.


In the fix the test is reworked to use BufferedImage instead of Frame, 
so it will work in headful and headless environment.


Some additional info:
http://mail.openjdk.java.net/pipermail/2d-dev/2018-February/008947.html

Re: [OpenJDK 2D-Dev] [10] Review Request: 8182410, 8183508, 8181289

[Semyon Sadetsky]

> The current changes get my "+1" as comformant HTML5.

This is not true.  The empty  tags produce warnings in HTML5 validator:

https://validator.w3.org/nu/?doc=http%3A%2F%2Fcr.openjdk.java.net%2F~ssadetsky%2F8181289%2Fjdoc%2FmodB-summary.html




-phil.

On 11/22/2017 07:58 PM, Jonathan Gibbons wrote:


Semyon,

You may indeed have explained why the behavior as it is, but we 
cannot and should not link this review with changes to the javadoc 
stylesheets, when the specific changes in the review are gratuitous 
and not necessary in the first place.


Yes, we may separately, and later, look at how the javadoc manages 
the header. Until then, I recommend that we stay within guidelines 
that are fully conformant HTML5, and without visual issues with the 
existing stylesheet.


So, I want to end this back and forth. I've spent enough time on 
this. I've given my review feedback, which remains to not introduce 
changes which may cause visual issues. If you and the  AWT team want 
to proceed with those changes, I'm done.


-- Jon

On 11/22/17 7:46 PM, Semyon Sadetsky wrote:


Jon,

This is because you have fixed page header. For me it works equally 
in all browsers. I see no discrepancy between Chrome and Firefox on 
my Linux platform. I believe that the stylesheet.css you have in 
those examples does the magic :


a[name]:before, a[name]:target, a[id]:before, a[id]:target {
    content: "";
    display: inline-block;
    position: relative;
    padding-top: 129px;
    margin-top: -129px;
}

so nothing specific comes from browser or "

I think the next link may help you

http://nicolasgallagher.com/jump-links-and-viewport-positioning/demo/

--Semyon


On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:

Semyon,

I have reconstructed a very simple, very artificial example to demo 
the bug.   This example uses lots of filler text, but while that is 
artificial, for sake of recreating a demo,  note that the problem 
first appeared, for real, in real JDK 9 API documentation with 
extended doc comments, and that as a result, we followed the advice 
I have been trying to give you.


See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc 
comments, with a heading at the top and a link at the bottom.


In modA, the anchor is of the form .  In modB, the 
anchor is of the form .


In each of these files, scroll to the end of the comment, and look 
for a link, called "link", at the bottom of the page.  In both 
cases, the page scrolls so that the heading is near the top of the 
browser window, but in one case it is hidden under the javadoc 
navbar, and in the other case, it is clearly visible, below the 
javadoc navbar.


This is the difference in behavior that I can been trying to 
describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm 
not the only one to have seen this effect.  I don't know whether 
you will get the same effect in your browser, but the fact that 
there is a reasonable OS/browser combo that demonstrates the 
problem is enough of a reason to avoid provoking the problem 
unnecessarily.   If you don't see the problem on your browser, but 
want to see it in mine, I see you are in SCA22, so drop by my 
office for a demo.


I'll leave it to the AWT team to decide what to do about this 
bug/review. I still recommend updating what is necessary to fix 
issues, and not otherwise changing the doc comments unnecessarily, 
and not changing them in a way to provoke this bad behavior.


-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:


Hi Jon,

This is not only about HTML5 spec, I also hardly can find 
resources that follow your "

You are correct that there is no bug here. But a bug was absent  
before this fix as well. This bug is about following to the HTML5 
standards, so let's follow them in full and not to return to this 
once again. We have a good chance to provide documentation in 
clean HTML5 after the fix without any workarounds.


--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:


Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc 
team) started using id on other elements, as well as  to 
provide a target that could be linked to.


However, the pragmatic experience was that the scrolling in some 
browsers did not completely reveal the element when there was a 
layered z component involved: the target element sometimes ended 
up under that layered component. Our experience was that the 
behavior was fixed when the target identifier was in an  element.


So, yes, you can follow the rules, and suggest that it is OK to 
put id on any element, and use it as a fragment identifier in a 
link, as given in the spec. Or you can be nice to your readers, 
and workaround what is probably a display bug in some browsers.


In the case of this review, you were suggesting additional 
"cleanup" on code that worked. Since 

Re: [OpenJDK 2D-Dev] [10] Review Request: 8182410, 8183508, 8181289

[Semyon Sadetsky]

Jon,

You shouldn't use empty  tag for margins. It looks odd. You can add 
the same css rule for appropriate element. I don't understand which 
visual issues you mean.


--Semyon

On 11/22/2017 07:58 PM, Jonathan Gibbons wrote:


Semyon,

You may indeed have explained why the behavior as it is, but we cannot 
and should not link this review with changes to the javadoc 
stylesheets, when the specific changes in the review are gratuitous 
and not necessary in the first place.


Yes, we may separately, and later, look at how the javadoc manages the 
header. Until then, I recommend that we stay within guidelines that 
are fully conformant HTML5, and without visual issues with the 
existing stylesheet.


So, I want to end this back and forth. I've spent enough time on this. 
I've given my review feedback, which remains to not introduce changes 
which may cause visual issues. If you and the AWT team want to proceed 
with those changes, I'm done.


-- Jon

On 11/22/17 7:46 PM, Semyon Sadetsky wrote:


Jon,

This is because you have fixed page header. For me it works equally 
in all browsers. I see no discrepancy between Chrome and Firefox on 
my Linux platform. I believe that the stylesheet.css you have in 
those examples does the magic :


a[name]:before, a[name]:target, a[id]:before, a[id]:target {
    content: "";
    display: inline-block;
    position: relative;
    padding-top: 129px;
    margin-top: -129px;
}

so nothing specific comes from browser or "

I think the next link may help you

http://nicolasgallagher.com/jump-links-and-viewport-positioning/demo/

--Semyon


On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:

Semyon,

I have reconstructed a very simple, very artificial example to demo 
the bug.   This example uses lots of filler text, but while that is 
artificial, for sake of recreating a demo,  note that the problem 
first appeared, for real, in real JDK 9 API documentation with 
extended doc comments, and that as a result, we followed the advice 
I have been trying to give you.


See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc 
comments, with a heading at the top and a link at the bottom.


In modA, the anchor is of the form .  In modB, the 
anchor is of the form .


In each of these files, scroll to the end of the comment, and look 
for a link, called "link", at the bottom of the page.  In both 
cases, the page scrolls so that the heading is near the top of the 
browser window, but in one case it is hidden under the javadoc 
navbar, and in the other case, it is clearly visible, below the 
javadoc navbar.


This is the difference in behavior that I can been trying to 
describe to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not 
the only one to have seen this effect.  I don't know whether you 
will get the same effect in your browser, but the fact that there is 
a reasonable OS/browser combo that demonstrates the problem is 
enough of a reason to avoid provoking the problem unnecessarily.   
If you don't see the problem on your browser, but want to see it in 
mine, I see you are in SCA22, so drop by my office for a demo.


I'll leave it to the AWT team to decide what to do about this 
bug/review. I still recommend updating what is necessary to fix 
issues, and not otherwise changing the doc comments unnecessarily, 
and not changing them in a way to provoke this bad behavior.


-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:


Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources 
that follow your "

You are correct that there is no bug here. But a bug was absent  
before this fix as well. This bug is about following to the HTML5 
standards, so let's follow them in full and not to return to this 
once again. We have a good chance to provide documentation in clean 
HTML5 after the fix without any workarounds.


--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:


Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc 
team) started using id on other elements, as well as  to 
provide a target that could be linked to.


However, the pragmatic experience was that the scrolling in some 
browsers did not completely reveal the element when there was a 
layered z component involved: the target element sometimes ended 
up under that layered component. Our experience was that the 
behavior was fixed when the target identifier was in an  element.


So, yes, you can follow the rules, and suggest that it is OK to 
put id on any element, and use it as a fragment identifier in a 
link, as given in the spec. Or you can be nice to your readers, 
and workaround what is probably a display bug in some browsers.


In the case of this review, you were suggesting additional 
"cleanup" on code that worked. Since there was no bug involved, 
and thus no inherent need to fix the code, my review feedback is 

Re: [OpenJDK 2D-Dev] [10] Review Request: 8182410, 8183508, 8181289

[Semyon Sadetsky]

Jon,

This is because you have fixed page header. For me it works equally in 
all browsers. I see no discrepancy between Chrome and Firefox on my 
Linux platform. I believe that the stylesheet.css you have in those 
examples does the magic :


a[name]:before, a[name]:target, a[id]:before, a[id]:target {
    content: "";
    display: inline-block;
    position: relative;
    padding-top: 129px;
    margin-top: -129px;
}

so nothing specific comes from browser or "

I think the next link may help you

http://nicolasgallagher.com/jump-links-and-viewport-positioning/demo/

--Semyon


On 11/22/2017 02:53 PM, Jonathan Gibbons wrote:

Semyon,

I have reconstructed a very simple, very artificial example to demo 
the bug.   This example uses lots of filler text, but while that is 
artificial, for sake of recreating a demo,  note that the problem 
first appeared, for real, in real JDK 9 API documentation with 
extended doc comments, and that as a result, we followed the advice I 
have been trying to give you.


See the toy API bundle here:
http://cr.openjdk.java.net/~jjg/semyon/api/overview-summary.html

There are two modules, modA and modB.  Both have huge long doc 
comments, with a heading at the top and a link at the bottom.


In modA, the anchor is of the form .  In modB, the 
anchor is of the form .


In each of these files, scroll to the end of the comment, and look for 
a link, called "link", at the bottom of the page.  In both cases, the 
page scrolls so that the heading is near the top of the browser 
window, but in one case it is hidden under the javadoc navbar, and in 
the other case, it is clearly visible, below the javadoc navbar.


This is the difference in behavior that I can been trying to describe 
to you. I'm using Ubuntu 14.04 with Firefox 38, but I'm not the only 
one to have seen this effect.  I don't know whether you will get the 
same effect in your browser, but the fact that there is a reasonable 
OS/browser combo that demonstrates the problem is enough of a reason 
to avoid provoking the problem unnecessarily.   If you don't see the 
problem on your browser, but want to see it in mine, I see you are in 
SCA22, so drop by my office for a demo.


I'll leave it to the AWT team to decide what to do about this 
bug/review. I still recommend updating what is necessary to fix 
issues, and not otherwise changing the doc comments unnecessarily, and 
not changing them in a way to provoke this bad behavior.


-- Jon




On 11/22/2017 12:10 PM, Semyon Sadetsky wrote:


Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources 
that follow your "

You are correct that there is no bug here. But a bug was absent  
before this fix as well. This bug is about following to the HTML5 
standards, so let's follow them in full and not to return to this 
once again. We have a good chance to provide documentation in clean 
HTML5 after the fix without any workarounds.


--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:


Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) 
started using id on other elements, as well as  to provide a 
target that could be linked to.


However, the pragmatic experience was that the scrolling in some 
browsers did not completely reveal the element when there was a 
layered z component involved: the target element sometimes ended up 
under that layered component. Our experience was that the behavior 
was fixed when the target identifier was in an  element.


So, yes, you can follow the rules, and suggest that it is OK to put 
id on any element, and use it as a fragment identifier in a link, as 
given in the spec. Or you can be nice to your readers, and 
workaround what is probably a display bug in some browsers.


In the case of this review, you were suggesting additional "cleanup" 
on code that worked. Since there was no bug involved, and thus no 
inherent need to fix the code, my review feedback is to leave the 
code alone.  You may choose to insist differently, and I cannot say 
that what you are suggesting is against the spec; I can just say 
that we can seen cases where such changes leads to bad visual effects.


-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:


Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:


Semyon,

Although id is a global attribute and can be used to identify any 
node, some browsers do better navigation/scrolling when the id is 
in an  tag. We have seen poor autoscrolling behavior when the 
id is an a header tag, such that the header ends up obscured under 
the navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is 
something different. Do you have any references those issues 
reports? Because in html5 the fragment identifiers are the only 
correct way to have internal document bookmarks [1] [2]. If some 
browsers do not navigate to fragment identifiers except for  
element there must be bugs 

Re: [OpenJDK 2D-Dev] [10] Review Request: 8182410, 8183508, 8181289

[Semyon Sadetsky]

Hi Jon,

This is not only about HTML5 spec, I also hardly can find resources that 
follow your "

You are correct that there is no bug here. But a bug was absent before 
this fix as well. This bug is about following to the HTML5 standards, so 
let's follow them in full and not to return to this once again. We have 
a good chance to provide documentation in clean HTML5 after the fix 
without any workarounds.


--Semyon

On 11/14/2017 09:16 AM, Jonathan Gibbons wrote:


Semyon,

I read the HTML 5 spec the same as you, and we (on the Javadoc team) 
started using id on other elements, as well as  to provide a target 
that could be linked to.


However, the pragmatic experience was that the scrolling in some 
browsers did not completely reveal the element when there was a 
layered z component involved: the target element sometimes ended up 
under that layered component. Our experience was that the behavior was 
fixed when the target identifier was in an  element.


So, yes, you can follow the rules, and suggest that it is OK to put id 
on any element, and use it as a fragment identifier in a link, as 
given in the spec. Or you can be nice to your readers, and workaround 
what is probably a display bug in some browsers.


In the case of this review, you were suggesting additional "cleanup" 
on code that worked. Since there was no bug involved, and thus no 
inherent need to fix the code, my review feedback is to leave the code 
alone.  You may choose to insist differently, and I cannot say that 
what you are suggesting is against the spec; I can just say that we 
can seen cases where such changes leads to bad visual effects.


-- Jon


On 10/25/17 6:31 PM, Semyon Sadetsky wrote:


Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:


Semyon,

Although id is a global attribute and can be used to identify any 
node, some browsers do better navigation/scrolling when the id is in 
an  tag.  We have seen poor autoscrolling behavior when the id is 
an a header tag, such that the header ends up obscured under the 
navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is 
something different. Do you have any references those issues reports? 
Because in html5 the fragment identifiers are the only correct way to 
have internal document bookmarks [1] [2]. If some browsers do not 
navigate to fragment identifiers except for  element there must be 
bugs reported that  which will be fixed soon.
The html5 specification is very specific about navigating to the 
fragment identifier [3]. So, there should no be difference between 
navigating to "

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid



-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:

Hi Sergey,

I see no reason to have an extra empty anchor tag to set a 
bookmark. The id attribute works with any element.


For example:

    
    Definitions

should be

    Definitions

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:


Hello,
Please review the fix for.
8182410: missing 'title' in 
api/javax/swing/plaf/synth/doc-files/componentProperties.html

8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like , , 
font, etc.(except the tags related to tables which I'll fix later).

 - HTML5 doctype is set for all files.
 - The  is set for all files.
 - 
Why you replace


 - Copyrights were added to some files.

Note that I placed a  tag before copyright to solve errors 
like:
"A charset attribute on a meta element found after the first 1024 
bytes. Fatal Error: Changing encoding at this point would need 
non-streamable behavior"


specdiff: 
http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html 



Bugs:
https://bugs.openjdk.java.net/browse/JDK-8182410
https://bugs.openjdk.java.net/browse/JDK-8183508
https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8181289/webrev.00

Re: [OpenJDK 2D-Dev] [10] Review Request: 8182410, 8183508, 8181289

[Semyon Sadetsky]

Hi Jonathan,


On 10/24/2017 03:20 PM, Jonathan Gibbons wrote:


Semyon,

Although id is a global attribute and can be used to identify any 
node, some browsers do better navigation/scrolling when the id is in 
an  tag.  We have seen poor autoscrolling behavior when the id is 
an a header tag, such that the header ends up obscured under the 
navigation bar at the top of the page.
You probably meant heading elements, because "header tag" is something 
different. Do you have any references those issues reports? Because in 
html5 the fragment identifiers are the only correct way to have internal 
document bookmarks [1] [2]. If some browsers do not navigate to fragment 
identifiers except for  element there must be bugs reported that  
which will be fixed soon.
The html5 specification is very specific about navigating to the 
fragment identifier [3]. So, there should no be difference between 
navigating to "

--Semyon

[1] https://www.w3schools.com/html/html_links.asp
[2] http://www.html5-tutorials.org/html-basics/links/
[3] https://www.w3.org/TR/html5/browsers.html#scroll-to-fragid



-- Jon


On 10/23/2017 10:08 PM, Semyon Sadetsky wrote:

Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. 
The id attribute works with any element.


For example:

    
    Definitions

should be

    Definitions

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:


Hello,
Please review the fix for.
8182410: missing 'title' in 
api/javax/swing/plaf/synth/doc-files/componentProperties.html

8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like , , 
font, etc.(except the tags related to tables which I'll fix later).

 - HTML5 doctype is set for all files.
 - The  is set for all files.
 - 
Why you replace


 - Copyrights were added to some files.

Note that I placed a  tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 
bytes. Fatal Error: Changing encoding at this point would need 
non-streamable behavior"


specdiff: 
http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html


Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8181289/webrev.00

Re: [OpenJDK 2D-Dev] [10] Review Request: 8182410, 8183508, 8181289

[Semyon Sadetsky]

On 10/24/2017 12:02 PM, Sergey Bylokhov wrote:


Hi, Semyon
I guess I did not add a new  tags, it seems that this tag is used 
as an anchor everywhere and since it is allowed in html5 its usage was 
preserved.
Starting version 9 JDK supports the html5 markup in javadoc, so the 
files need to be updated according to it. In html5 you need  only if 
you want to see hyperlink and your fix is modifying those tags.


--Semyon


On 23/10/2017 22:08, Semyon Sadetsky wrote:

Hi Sergey,

I see no reason to have an extra empty anchor tag to set a bookmark. 
The id attribute works with any element.


For example:

 
 Definitions

should be

 Definitions

--Semyon

On 10/23/2017 02:42 PM, Sergey Bylokhov wrote:


Hello,
Please review the fix for.
8182410: missing 'title' in 
api/javax/swing/plaf/synth/doc-files/componentProperties.html

8183508: multi_tsc.html should be updated
8181289: Invalid HTML 5 in AWT/Swing docs

Description:
 - Illegal characters were removed.
 - Unsupported tags/properties were removed -like , , 
font, etc.(except the tags related to tables which I'll fix later).

 - HTML5 doctype is set for all files.
 - The  is set for all files.
 - 
Why you replace


 - Copyrights were added to some files.

Note that I placed a  tag before copyright to solve errors like:
"A charset attribute on a meta element found after the first 1024 
bytes. Fatal Error: Changing encoding at this point would need 
non-streamable behavior"


specdiff: 
http://cr.openjdk.java.net/~serb/8181289/specdiff/overview-summary.html


Bugs:
    https://bugs.openjdk.java.net/browse/JDK-8182410
    https://bugs.openjdk.java.net/browse/JDK-8183508
    https://bugs.openjdk.java.net/browse/JDK-8181289

Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8181289/webrev.00

Re: [OpenJDK 2D-Dev] RFR: 8080084 : java/awt/Graphics2D/DrawString/DrawStringCrash.java fails with G1 GC

[Semyon Sadetsky]

+1

--Semyon


On 10/09/2017 11:09 AM, Phil Race wrote:

A test fix .. to catch OOME from StringBuilder.append() as well
as the API that is under test.

I don't think there's anything specific to G1 here, it could
have happened with other collectors.

bug : https://bugs.openjdk.java.net/browse/JDK-8080084
webrev: http://cr.openjdk.java.net/~prr/8080084/

-phil.

Re: [OpenJDK 2D-Dev] [10] Review Request 8187367: Numerous swing display problems with scaled displays on Windows

[Semyon Sadetsky]

On 09/18/2017 12:01 PM, Sergey Bylokhov wrote:

Hi, Semyon.
Is it possible to write a test case for this issue? It will be helpful 
to understand the bug and a fix.


Writing the test is hard because those inner interfaces are  too deep 
inside. You can use SwingSet2 demo to see the described artifacts at 
fractional scales before the fix.




On 9/18/17 09:36, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK10 (in Swing and Java2D):

bug: https://bugs.openjdk.java.net/browse/JDK-8187367

webrev: http://cr.openjdk.java.net/~ssadetsky/8187367/webrev.00/

Swing apps  may have artifacts on HiDPI screens with fractional 
scales. There are several issues which may cause various artifacts 
but the current fix only eliminates one type of artifacts the 
vertical and horizontal lines on painted surfaces (other issues are 
addressed by [1] and [2]). This issue was introduced after 8073320 
and then incorrectly fixed in 8163193.


The root cause is the painter pattern is drawn on a wrongly sized and 
scaled image in case the image is an off-screen volatile image. The 
painter doesn't take into account the volatile image inner scale 
transformation which is not an identity in case of HiDPI , so the 
resulting image is drawn on of off-screen surface that in the scale 
times bigger than it is actually necessary. Since such images are 
cached this was waste of RAM. Also, the bounds sent to the painter 
class don't take into account the transformation rounding error the 
latter caused artifacts on the drawing edges.


--Semyon

[1] https://bugs.openjdk.java.net/browse/JDK-8187585

[2] https://bugs.openjdk.java.net/browse/JDK-8187586

[OpenJDK 2D-Dev] [10] Review Request 8187367: Numerous swing display problems with scaled displays on Windows

[Semyon Sadetsky]

Hello,

Please review fix for JDK10 (in Swing and Java2D):

bug: https://bugs.openjdk.java.net/browse/JDK-8187367

webrev: http://cr.openjdk.java.net/~ssadetsky/8187367/webrev.00/

Swing apps  may have artifacts on HiDPI screens with fractional scales. 
There are several issues which may cause various artifacts but the 
current fix only eliminates one type of artifacts the vertical and 
horizontal lines on painted surfaces (other issues are addressed by [1] 
and [2]). This issue was introduced after 8073320 and then incorrectly 
fixed in 8163193.


The root cause is the painter pattern is drawn on a wrongly sized and 
scaled image in case the image is an off-screen volatile image. The 
painter doesn't take into account the volatile image inner scale 
transformation which is not an identity in case of HiDPI , so the 
resulting image is drawn on of off-screen surface that in the scale 
times bigger than it is actually necessary. Since such images are cached 
this was waste of RAM. Also, the bounds sent to the painter class don't 
take into account the transformation rounding error the latter caused 
artifacts on the drawing edges.


--Semyon

[1] https://bugs.openjdk.java.net/browse/JDK-8187585

[2] https://bugs.openjdk.java.net/browse/JDK-8187586

Re: [OpenJDK 2D-Dev] [9] Review Request for 8159902: OGL surfaces are not HiDPI compatible on Linux/Solaris

[Semyon Sadetsky]

On 04/30/2017 07:55 PM, Sergey Bylokhov wrote:


Hi, Semyon.
Should the GLXVSyncOffScreenSurfaceData/WGLVSyncOffScreenSurfaceData be updated 
as well? Seems it were also overlooked.
I thinks that instead of «noreg-sqe» label it would be better to add the new 
bugid to the failed test.
The GLXVSyncOffScreenSurfaceData surface will inherit this functionality 
from the GLXOffScreenSurfaceData class after the current fix. The 
WGLVSyncOffScreenSurfaceData surface inherits HiDPI support from the 
WGLOffScreenSurfaceData class.


--Semyon

Hello,

Please review the fix for jdk9.

bug:https://bugs.openjdk.java.net/browse/JDK-8159902

webrev: http://cr.openjdk.java.net/~ssadetsky/8159902/webrev.00/

Probably, HiDPI support for Linux OGL was forgoten.

Fix tries to add it now...

--Semyon

Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor issues in awt coding

[Semyon Sadetsky]

yes, this is a potential issue. But actually cases MID,FOCUS,BLACK,WHITE 
are never used.


But the fix is wrong. It should be

color.alpha = 1; --Semyon


On 01.12.2016 08:05, Prasanta Sadhukhan wrote:


Also, in gtk3_interface.c, there is this change for color.alpha

2219 color.alpha = 0; in gtk3_get_color_for_flags()
but it is used in
gtk3_get_color_for_state() where it is not initialized

2268 GdkRGBA color;
Regards
Prasanta
On 12/1/2016 1:28 AM, Phil Race wrote:

Hi Goetz,


  DataBufferNative.c
Using uninitialized value lockInfo.rasBase when calling 
DBN_GetPixelPointer.


  75 lockInfo.resBase = NULL;

Did you actually compile this ? The variable is called "rasBase", not 
"resBase".


And strictly there is no problem since inside DBN_GetPixelPointer
the code calls ops->Lock which should initialise this.
A "rasBase" of 0 isn't really any better than a random one ..

Also I don't see why there's a problem here and not in
the function immediately following since it is the exact same case.

-phil.

On 11/30/2016 10:00 AM, Sergey Bylokhov wrote:

cc 2d-dev.

On 30.11.16 18:41, Lindenmaier, Goetz wrote:

Hi Vincent,

thanks for the quit review!
Good catch that I lost the change to p11_mutex.c ... I had to change
it and it fell out of my patches.
I edited the Last Modified Date, and also updated the copyright 
messages.

New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/

Best regards,
  Goetz.

(Am I correct that your openJdk name is Vinnie?)


-Original Message-
From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 14:53
To: Lindenmaier, Goetz 
Cc: awt-...@openjdk.java.net; security-...@openjdk.java.net
Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding

Hello Goetz,

Please modify the bug summary to reference ECC too.
Your ECC changes look fine but the ‘Last Modified Date’ line in 
the 4 source

code headers will need to be updated/added.

BTW p11_mutex.c is listed below but appears to be missing from the 
webrev.


Thanks.




On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
 > 
wrote:


Hi,

I’d like to propose a row of smaller fixes where code is noted 
down a

bit questionable.
SAP’s quality process requires that we fix these in our 
internal delivery,

and I
Would like to share my fixes with openJdk.  Some of these 
fixes are of

more
theoretical nature as how I understand the code paths never 
allow the
problematic situation, but fixing it nevertheless assures that 
nothing is
overseen if the code changes.  Most changes are in 
libawt_xawt, some

are in libsunec.

I’d appreciate a review:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/

Changes in detail:

awt_InputMethod.c:

One might overrun the 100 byte fixed-size string 
statusWindow->status

by copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never 
true.


Using uninitialized value color. Field color.alpha is 
uninitialized.

E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string 
conversion.


splashscreen_sys.c

Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by 
copying

curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling 
*group->point_mul. (The

function pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized 
when calling

s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized 
when

calling s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field 
ckpInitArgs->flags is

uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling
BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath 
by copying

DirP->name[index] without checking the length.

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

Thanks! I found this setting very useful.

--Semyon

On 9/30/2016 6:51 AM, Philip Race wrote:


BTW I just noticed in the source code that there is an
 environment variable that disables the check :

see windows/native/libawt/java2d/d3d/D3DPipelineManager.cpp
static BOOL bNoHwCheck = (getenv("J2D_D3D_NO_HWCHECK") != NULL);

if that is set it does everything except return the code that means this
is bad hardware .. seems Dmitri added this from the beginning.

I overlooked this in the past because I was focused on the system 
property settings.


-phil.


On 9/9/16, 8:14 AM, Philip Race wrote:
No .. that would be an incompatible change that might surprise a lot 
of people.


-phil.

On 9/9/16, 12:31 AM, Semyon Sadetsky wrote:
I cannot reproduce JDK-803944. It is about very specific hardware 
configuration with two different video cards.


I didn't find any evaluation/justification, neither in JIRA nor in 
the review on the alias, for the 803944 resolution that d3d should 
be disabled for all Intel video cards. Why?


It may be disabled by default, but at least the sun.java2d.d3d=true 
option could enable it, no?


--Semyon


On 9/9/2016 4:58 AM, Philip Race wrote:

Please consider https://bugs.openjdk.java.net/browse/JDK-8039444
and the various bugs that were closed as a duplicate of that bug.
I don't think you can easily show this fix resolves all of these ..

-phil.


On 9/8/16, 5:12 PM, Semyon Sadetsky wrote:
I have 2 laptops Intel i5, i7. Both working with d3d normally. 
Some visual defects will be corrected after this fix.
And I didn't get why d3d is disabled for all Intel without 
possibility to switch it on?


--Semyon

On 09.09.2016 02:10, Philip Race wrote:
The following is just for testing right ? It should not be in 
this webrev

as part of what you propose to push ..
--
src/java.desktop/windows/native/libawt/java2d/d3d/D3DBadHardware.h
Print this page

@@ -49,13 +49,10 @@
 // all versions must fail ("there's no version of the driver 
that passes")

 #define NO_VERSION D_VERSION(0x, 0x, 0x, 0x)

 static const ADAPTER_INFO badHardware[] = {

-// All Intel Chips.
-{ 0x8086, ALL_DEVICEIDS, NO_VERSION, OS_ALL },
-
 // ATI Mobility Radeon X1600, X1400, X1450, X1300, X1350
 // Reason: workaround for 6613066, 6687166
 // X1300 (four sub ids)
 { 0x1002, 0x714A, D_VERSION(6,14,10,6706), OS_WINXP },
 { 0x1002, 0x714A, D_VERSION(7,14,10,0567), OS_VISTA },

---

-phil.

On 9/8/16, 4:06 PM, Semyon Sadetsky wrote:

I have reworked fix to not affect ATI and NVidia.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.05/

--Semyon


On 9/9/2016 12:20 AM, Semyon Sadetsky wrote:



On 08.09.2016 22:57, Philip Race wrote:


Can you provide something like a rationale for why these 
particular values might work ?
Otherwise this seems like a fix that can't be reviewed, only 
tested.

So that testing will be important. If you can be sure it passes
on ATI, Nvidia, and Intel then we can take it .. otherwise we 
should defer this.
I suppose those fudge factors are obtained experimentally. Not 
sure that any rationale is possible here.
The fix simply tested on different hardware. I hope after this 
fix D3D maybe enabled again for Intel APUs.

Currently it has been blacklisted in 8039444.

--Semyon


IIRC Semyon will need to change the code to bypass the check
for Intel hardware. There is no env. variable or system 
property to do this.


-phil.

On 9/8/16, 3:47 AM, Sergey Bylokhov wrote:

On 05.09.16 13:36, Semyon Sadetsky wrote:

At last I could get NVidia machine (special thanks to Yuri).

The updated fix should improve the situation on NVidia. For 
that one
common height/width fudge factor was separated in two 
different.


http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.04/


Can you please confirm that the fix and test works if d3d is 
enabled on the intel vk? I recall that d3d was disabled on 
intel so probably to check that we need to force d3d manually.



On 3/18/2016 9:12 AM, Semyon Sadetsky wrote:

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with 
fudge factors

values but I don't have nvidia based video to test.

--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having 
windows build

problems and
it passed on my ATI card.

-phil.


On 03/01/2016 04:37 PM, Sergey Bylokhov wrote:

On 15.01.16 9:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other 
hardware.
On Mac's retina display the screen capture doesn't 
return exact

pixel to
pixel image but the scaled one. So Mac platform should 
be excluded

from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


I run the latest test(webrev.03) on my nvidia card, and 
it fails
after the fix, but pass before =(. I have no ati to 
check. Also I
cannot check the fix on intel card, because I cannot 
enable d3d on it.




--Semyon

On 1/14/2016 

Re: [OpenJDK 2D-Dev] [9] Review Request 8155753: Removing a monitor in the OS dispaly configuration causes assertion fails under Windows

[Semyon Sadetsky]

 I missed this file in the list of changes.

http://cr.openjdk.java.net/~ssadetsky/8155753/webrev.02/

--Semyon


On 9/12/2016 9:33 PM, Vadim Pakhnushev wrote:

Have you forgotten adding changes in AccelGraphicsConfig?

c:\Vadim\jdk9-client\jdk\src\java.desktop\windows\classes\sun\java2d\d3d\D3DGraphicsConfig.java:52: 
error: D3DGraphicsConfig is not abstract and does not override 
abstract method removeDeviceEventListener(AccelDeviceEventListener) in 
AccelGraphicsConfig

public class D3DGraphicsConfig
   ^
c:\Vadim\jdk9-client\jdk\src\java.desktop\windows\classes\sun\java2d\opengl\WGLGraphicsConfig.java:59: 
error: WGLGraphicsConfig is not abstract and does not override 
abstract method removeDeviceEventListener(AccelDeviceEventListener) in 
AccelGraphicsConfig

public class WGLGraphicsConfig
   ^

Also +#include "Devices.h" in the D3DContext.cpp is a leftover.

Vadim

On 12.09.2016 21:11, Semyon Sadetsky wrote:

http://cr.openjdk.java.net/~ssadetsky/8155753/webrev.01/

AccelDeviceEventNotifier is removed.

--Semyon


On 9/12/2016 6:56 PM, Semyon Sadetsky wrote:

Okay. I will remove AccelDeviceEventNotifier and all related code.

--Semyon


On 9/12/2016 6:43 PM, Vadim Pakhnushev wrote:

Hi Semyon,

Generally seems reasonable, it seems that you should use screen 
instead of gdiScreen in the JNU_CallStaticMethodByName, otherwise 
the code won't compile.
Not sure how the rest of the code handles monitor removal, seems to 
me that there are no usages of this notifications anywhere, so 
maybe we don't need this code at all?


Thanks,
Vadim

On 12.09.2016 17:24, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8155753

webrev: http://cr.openjdk.java.net/~ssadetsky/8155753/webrev.00/

The issue take place on Windows platform if Direct3d is on. The 
notification routine about the monitor removal tries to get screen 
number using the monitor handle which is obviously null at this 
moment. As a fix the screen number is recorded in D3D context for 
further possible notifications.


--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request 8155753: Removing a monitor in the OS dispaly configuration causes assertion fails under Windows

[Semyon Sadetsky]

http://cr.openjdk.java.net/~ssadetsky/8155753/webrev.01/

AccelDeviceEventNotifier is removed.

--Semyon


On 9/12/2016 6:56 PM, Semyon Sadetsky wrote:

Okay. I will remove AccelDeviceEventNotifier and all related code.

--Semyon


On 9/12/2016 6:43 PM, Vadim Pakhnushev wrote:

Hi Semyon,

Generally seems reasonable, it seems that you should use screen 
instead of gdiScreen in the JNU_CallStaticMethodByName, otherwise the 
code won't compile.
Not sure how the rest of the code handles monitor removal, seems to 
me that there are no usages of this notifications anywhere, so maybe 
we don't need this code at all?


Thanks,
Vadim

On 12.09.2016 17:24, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8155753

webrev: http://cr.openjdk.java.net/~ssadetsky/8155753/webrev.00/

The issue take place on Windows platform if Direct3d is on. The 
notification routine about the monitor removal tries to get screen 
number using the monitor handle which is obviously null at this 
moment. As a fix the screen number is recorded in D3D context for 
further possible notifications.


--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request 8155753: Removing a monitor in the OS dispaly configuration causes assertion fails under Windows

[Semyon Sadetsky]

Okay. I will remove AccelDeviceEventNotifier and all related code.

--Semyon


On 9/12/2016 6:43 PM, Vadim Pakhnushev wrote:

Hi Semyon,

Generally seems reasonable, it seems that you should use screen 
instead of gdiScreen in the JNU_CallStaticMethodByName, otherwise the 
code won't compile.
Not sure how the rest of the code handles monitor removal, seems to me 
that there are no usages of this notifications anywhere, so maybe we 
don't need this code at all?


Thanks,
Vadim

On 12.09.2016 17:24, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8155753

webrev: http://cr.openjdk.java.net/~ssadetsky/8155753/webrev.00/

The issue take place on Windows platform if Direct3d is on. The 
notification routine about the monitor removal tries to get screen 
number using the monitor handle which is obviously null at this 
moment. As a fix the screen number is recorded in D3D context for 
further possible notifications.


--Semyon

[OpenJDK 2D-Dev] [9] Review Request 8155753: Removing a monitor in the OS dispaly configuration causes assertion fails under Windows

[Semyon Sadetsky]

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8155753

webrev: http://cr.openjdk.java.net/~ssadetsky/8155753/webrev.00/

The issue take place on Windows platform if Direct3d is on. The 
notification routine about the monitor removal tries to get screen 
number using the monitor handle which is obviously null at this moment. 
As a fix the screen number is recorded in D3D context for further 
possible notifications.


--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 9/9/2016 4:08 PM, Vadim Pakhnushev wrote:

On 09.09.2016 15:58, Semyon Sadetsky wrote:

On 09.09.2016 14:45, Vadim Pakhnushev wrote:





My cards are HD Graphics 3000 (0x8086/0x0126) with 9.17.10.4101 and
ATI Radeon HD 5700 (0x1002/0x68b8) with driver 8.17.10.1333

ATI is unrelated to this fix. Could you upgrade to the latest driver
from https://downloadcenter.intel.com/search?keyword=hd+graphics+3000
and try to reproduce 803944 again?


Alas, the issue is still there even with the latest driver:
https://i.imgur.com/1kXVbaI.png
I definitely cannot reproduce this picture on my system. Maybe it 
take place for old Intel cards only. HD 3000 is  5+ years old and 
probably it does not fully support Windows 7 d3d. HD graphics 
declares support for dx11 starting from HD4000.
This bug and all related were resolved by disabling d3d for all Intel 
video cards. We could at least to defer those bugs and disable d3d in 
a separate bug as a temp solution. We are forgetting about Intel d3d 
forever?
Its a pity, because Intel APUs as main graphics card are rather 
popular. Ultrabooks rarely have discrete video.


We had a lot of reports on HD4000 as well, listed as duplicates of 
803944.

DX11 support is not relevant here at all since we are only use DX9.

Since your fix explicitly checks for Intel hardware, it doesn't make 
any sense to use it without unblocking them.
Moreover, I can't see how this bug (8146042) can be reproduced without 
modified build which unblocks Intel hardware.
This may be explained. D3D on Intel was disabled in April while this bug 
fix was published in January. Before it was rather annoying. But it 
seems 803944 fix has resolved all all such issues for all Intel cards at 
once. And I have lost possibility to reproduce d3d related issues 
because I have no discrete card and even an option to force d3d is not 
provided.


--Semyon


Vadim

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 09.09.2016 14:45, Vadim Pakhnushev wrote:





My cards are HD Graphics 3000 (0x8086/0x0126) with 9.17.10.4101 and
ATI Radeon HD 5700 (0x1002/0x68b8) with driver 8.17.10.1333

ATI is unrelated to this fix. Could you upgrade to the latest driver
from https://downloadcenter.intel.com/search?keyword=hd+graphics+3000
and try to reproduce 803944 again?


Alas, the issue is still there even with the latest driver:
https://i.imgur.com/1kXVbaI.png
I definitely cannot reproduce this picture on my system. Maybe it take 
place for old Intel cards only. HD 3000 is  5+ years old and probably it 
does not fully support Windows 7 d3d. HD graphics declares support for 
dx11 starting from HD4000.
This bug and all related were resolved by disabling d3d for all Intel 
video cards. We could at least to defer those bugs and disable d3d in a 
separate bug as a temp solution. We are forgetting about Intel d3d forever?
Its a pity, because Intel APUs as main graphics card are rather popular. 
Ultrabooks rarely have discrete video.


--Semyon


Vadim

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 09.09.2016 13:43, Vadim Pakhnushev wrote:

On 09.09.2016 13:12, Semyon Sadetsky wrote:

On 09.09.2016 12:56, Vadim Pakhnushev wrote:


Semyon,

I can reproduce JDK-803944 on my machine with your fix.
More importantly, the RenderRectTest is passed for me on Windows 7 
on ATI and Intel cards _without the fix_.
My fix is not aimed to fix 803944. I cannot reproduce it. I have 
i5-4300M (HD Graphics 4600) and Intel driver 10.18.10.3412 signed by 
Microsoft. Which Intel card/APU and driver do you have?


You removed all Intel cards from blacklist and this reintroduced 
803944 since your fix doesn't address it.
My cards are HD Graphics 3000 (0x8086/0x0126) with 9.17.10.4101 and 
ATI Radeon HD 5700 (0x1002/0x68b8) with driver 8.17.10.1333
ATI is unrelated to this fix. Could you upgrade to the latest driver 
from https://downloadcenter.intel.com/search?keyword=hd+graphics+3000 
and try to reproduce 803944 again?


What about the RenderRectTest? In what configuration it fails?

It falls with d3d on Intel APUs.

--Semyon




Do you have a standalone test from the original report?

Simply run SwinSet2: all horizontal/vertical lines are shifted +1 px.


Could you please be more specific? Shifted from where to where? Maybe 
attach good/bad screenshots to the JIRA?


Thanks,
Vadim



--Semyon


Vadim

On 09.09.2016 10:31, Semyon Sadetsky wrote:
I cannot reproduce JDK-803944. It is about very specific hardware 
configuration with two different video cards.


I didn't find any evaluation/justification, neither in JIRA nor in 
the review on the alias, for the 803944 resolution that d3d should 
be disabled for all Intel video cards. Why?


It may be disabled by default, but at least the sun.java2d.d3d=true 
option could enable it, no?


--Semyon


On 9/9/2016 4:58 AM, Philip Race wrote:

Please consider https://bugs.openjdk.java.net/browse/JDK-8039444
and the various bugs that were closed as a duplicate of that bug.
I don't think you can easily show this fix resolves all of these ..

-phil.


On 9/8/16, 5:12 PM, Semyon Sadetsky wrote:
I have 2 laptops Intel i5, i7. Both working with d3d normally. 
Some visual defects will be corrected after this fix.
And I didn't get why d3d is disabled for all Intel without 
possibility to switch it on?


--Semyon

On 09.09.2016 02:10, Philip Race wrote:
The following is just for testing right ? It should not be in 
this webrev

as part of what you propose to push ..
--
src/java.desktop/windows/native/libawt/java2d/d3d/D3DBadHardware.h
Print this page

@@ -49,13 +49,10 @@
 // all versions must fail ("there's no version of the driver 
that passes")

 #define NO_VERSION D_VERSION(0x, 0x, 0x, 0x)

 static const ADAPTER_INFO badHardware[] = {

-// All Intel Chips.
-{ 0x8086, ALL_DEVICEIDS, NO_VERSION, OS_ALL },
-
 // ATI Mobility Radeon X1600, X1400, X1450, X1300, X1350
 // Reason: workaround for 6613066, 6687166
 // X1300 (four sub ids)
 { 0x1002, 0x714A, D_VERSION(6,14,10,6706), OS_WINXP },
 { 0x1002, 0x714A, D_VERSION(7,14,10,0567), OS_VISTA },

---

-phil.

On 9/8/16, 4:06 PM, Semyon Sadetsky wrote:

I have reworked fix to not affect ATI and NVidia.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.05/

--Semyon


On 9/9/2016 12:20 AM, Semyon Sadetsky wrote:



On 08.09.2016 22:57, Philip Race wrote:


Can you provide something like a rationale for why these 
particular values might work ?
Otherwise this seems like a fix that can't be reviewed, only 
tested.

So that testing will be important. If you can be sure it passes
on ATI, Nvidia, and Intel then we can take it .. otherwise we 
should defer this.
I suppose those fudge factors are obtained experimentally. Not 
sure that any rationale is possible here.
The fix simply tested on different hardware. I hope after this 
fix D3D maybe enabled again for Intel APUs.

Currently it has been blacklisted in 8039444.

--Semyon


IIRC Semyon will need to change the code to bypass the check
for Intel hardware. There is no env. variable or system 
property to do this.


-phil.

On 9/8/16, 3:47 AM, Sergey Bylokhov wrote:

On 05.09.16 13:36, Semyon Sadetsky wrote:

At last I could get NVidia machine (special thanks to Yuri).

The updated fix should improve the situation on NVidia. For 
that one
common height/width fudge factor was separated in two 
different.


http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.04/


Can you please confirm that the fix and test works if d3d is 
enabled on the intel vk? I recall that d3d was disabled on 
intel so probably to check that we need to force d3d manually.



On 3/18/2016 9:12 AM, Semyon Sadetsky wrote:

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with 
fudge factors

values but I don't have nvidia based video to test.

--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having 
windows build

problems and
it passed on my ATI card.

-phil.


On 03/01/201

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 09.09.2016 12:56, Vadim Pakhnushev wrote:


Semyon,

I can reproduce JDK-803944 on my machine with your fix.
More importantly, the RenderRectTest is passed for me on Windows 7 on 
ATI and Intel cards _without the fix_.
My fix is not aimed to fix 803944. I cannot reproduce it. I have 
i5-4300M (HD Graphics 4600) and Intel driver 10.18.10.3412 signed by 
Microsoft. Which Intel card/APU and driver do you have?


Do you have a standalone test from the original report?

Simply run SwinSet2: all horizontal/vertical lines are shifted +1 px.

--Semyon


Vadim

On 09.09.2016 10:31, Semyon Sadetsky wrote:
I cannot reproduce JDK-803944. It is about very specific hardware 
configuration with two different video cards.


I didn't find any evaluation/justification, neither in JIRA nor in 
the review on the alias, for the 803944 resolution that d3d should be 
disabled for all Intel video cards. Why?


It may be disabled by default, but at least the sun.java2d.d3d=true 
option could enable it, no?


--Semyon


On 9/9/2016 4:58 AM, Philip Race wrote:

Please consider https://bugs.openjdk.java.net/browse/JDK-8039444
and the various bugs that were closed as a duplicate of that bug.
I don't think you can easily show this fix resolves all of these ..

-phil.


On 9/8/16, 5:12 PM, Semyon Sadetsky wrote:
I have 2 laptops Intel i5, i7. Both working with d3d normally. Some 
visual defects will be corrected after this fix.
And I didn't get why d3d is disabled for all Intel without 
possibility to switch it on?


--Semyon

On 09.09.2016 02:10, Philip Race wrote:
The following is just for testing right ? It should not be in this 
webrev

as part of what you propose to push ..
--
src/java.desktop/windows/native/libawt/java2d/d3d/D3DBadHardware.h
Print this page

@@ -49,13 +49,10 @@
 // all versions must fail ("there's no version of the driver that 
passes")

 #define NO_VERSION D_VERSION(0x, 0x, 0x, 0x)

 static const ADAPTER_INFO badHardware[] = {

-// All Intel Chips.
-{ 0x8086, ALL_DEVICEIDS, NO_VERSION, OS_ALL },
-
 // ATI Mobility Radeon X1600, X1400, X1450, X1300, X1350
 // Reason: workaround for 6613066, 6687166
 // X1300 (four sub ids)
 { 0x1002, 0x714A, D_VERSION(6,14,10,6706), OS_WINXP },
 { 0x1002, 0x714A, D_VERSION(7,14,10,0567), OS_VISTA },

---

-phil.

On 9/8/16, 4:06 PM, Semyon Sadetsky wrote:

I have reworked fix to not affect ATI and NVidia.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.05/

--Semyon


On 9/9/2016 12:20 AM, Semyon Sadetsky wrote:



On 08.09.2016 22:57, Philip Race wrote:


Can you provide something like a rationale for why these 
particular values might work ?
Otherwise this seems like a fix that can't be reviewed, only 
tested.

So that testing will be important. If you can be sure it passes
on ATI, Nvidia, and Intel then we can take it .. otherwise we 
should defer this.
I suppose those fudge factors are obtained experimentally. Not 
sure that any rationale is possible here.
The fix simply tested on different hardware. I hope after this 
fix D3D maybe enabled again for Intel APUs.

Currently it has been blacklisted in 8039444.

--Semyon


IIRC Semyon will need to change the code to bypass the check
for Intel hardware. There is no env. variable or system 
property to do this.


-phil.

On 9/8/16, 3:47 AM, Sergey Bylokhov wrote:

On 05.09.16 13:36, Semyon Sadetsky wrote:

At last I could get NVidia machine (special thanks to Yuri).

The updated fix should improve the situation on NVidia. For 
that one

common height/width fudge factor was separated in two different.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.04/


Can you please confirm that the fix and test works if d3d is 
enabled on the intel vk? I recall that d3d was disabled on 
intel so probably to check that we need to force d3d manually.



On 3/18/2016 9:12 AM, Semyon Sadetsky wrote:

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with 
fudge factors

values but I don't have nvidia based video to test.

--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having windows 
build

problems and
it passed on my ATI card.

-phil.


On 03/01/2016 04:37 PM, Sergey Bylokhov wrote:

On 15.01.16 9:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other 
hardware.
On Mac's retina display the screen capture doesn't return 
exact

pixel to
pixel image but the scaled one. So Mac platform should be 
excluded

from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


I run the latest test(webrev.03) on my nvidia card, and it 
fails
after the fix, but pass before =(. I have no ati to check. 
Also I
cannot check the fix on intel card, because I cannot 
enable d3d on it.




--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before th

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

I cannot reproduce JDK-803944. It is about very specific hardware 
configuration with two different video cards.


I didn't find any evaluation/justification, neither in JIRA nor in the 
review on the alias, for the 803944 resolution that d3d should be 
disabled for all Intel video cards. Why?


It may be disabled by default, but at least the sun.java2d.d3d=true 
option could enable it, no?


--Semyon


On 9/9/2016 4:58 AM, Philip Race wrote:

Please consider https://bugs.openjdk.java.net/browse/JDK-8039444
and the various bugs that were closed as a duplicate of that bug.
I don't think you can easily show this fix resolves all of these ..

-phil.


On 9/8/16, 5:12 PM, Semyon Sadetsky wrote:
I have 2 laptops Intel i5, i7. Both working with d3d normally. Some 
visual defects will be corrected after this fix.
And I didn't get why d3d is disabled for all Intel without 
possibility to switch it on?


--Semyon

On 09.09.2016 02:10, Philip Race wrote:
The following is just for testing right ? It should not be in this 
webrev

as part of what you propose to push ..
--
src/java.desktop/windows/native/libawt/java2d/d3d/D3DBadHardware.h
Print this page

@@ -49,13 +49,10 @@
 // all versions must fail ("there's no version of the driver that 
passes")

 #define NO_VERSION D_VERSION(0x, 0x, 0x, 0x)

 static const ADAPTER_INFO badHardware[] = {

-// All Intel Chips.
-{ 0x8086, ALL_DEVICEIDS, NO_VERSION, OS_ALL },
-
 // ATI Mobility Radeon X1600, X1400, X1450, X1300, X1350
 // Reason: workaround for 6613066, 6687166
 // X1300 (four sub ids)
 { 0x1002, 0x714A, D_VERSION(6,14,10,6706), OS_WINXP },
 { 0x1002, 0x714A, D_VERSION(7,14,10,0567), OS_VISTA },

---

-phil.

On 9/8/16, 4:06 PM, Semyon Sadetsky wrote:

I have reworked fix to not affect ATI and NVidia.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.05/

--Semyon


On 9/9/2016 12:20 AM, Semyon Sadetsky wrote:



On 08.09.2016 22:57, Philip Race wrote:


Can you provide something like a rationale for why these 
particular values might work ?

Otherwise this seems like a fix that can't be reviewed, only tested.
So that testing will be important. If you can be sure it passes
on ATI, Nvidia, and Intel then we can take it .. otherwise we 
should defer this.
I suppose those fudge factors are obtained experimentally. Not 
sure that any rationale is possible here.
The fix simply tested on different hardware. I hope after this fix 
D3D maybe enabled again for Intel APUs.

Currently it has been blacklisted in 8039444.

--Semyon


IIRC Semyon will need to change the code to bypass the check
for Intel hardware. There is no env. variable or system property 
to do this.


-phil.

On 9/8/16, 3:47 AM, Sergey Bylokhov wrote:

On 05.09.16 13:36, Semyon Sadetsky wrote:

At last I could get NVidia machine (special thanks to Yuri).

The updated fix should improve the situation on NVidia. For 
that one

common height/width fudge factor was separated in two different.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.04/


Can you please confirm that the fix and test works if d3d is 
enabled on the intel vk? I recall that d3d was disabled on intel 
so probably to check that we need to force d3d manually.



On 3/18/2016 9:12 AM, Semyon Sadetsky wrote:

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with fudge 
factors

values but I don't have nvidia based video to test.

--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having windows 
build

problems and
it passed on my ATI card.

-phil.


On 03/01/2016 04:37 PM, Sergey Bylokhov wrote:

On 15.01.16 9:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return 
exact

pixel to
pixel image but the scaled one. So Mac platform should be 
excluded

from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


I run the latest test(webrev.03) on my nvidia card, and it 
fails
after the fix, but pass before =(. I have no ati to check. 
Also I
cannot check the fix on intel card, because I cannot enable 
d3d on it.




--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the 
comments

seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified 
hardware-dependent

territory but
it seems quite possible that this could just as easily 
introduce a

different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to 
include at

least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

bug: https://bugs.openjdk.java.net/browse/JDK-8146042
webrev: 
http://cr.op

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

I have 2 laptops Intel i5, i7. Both working with d3d normally. Some 
visual defects will be corrected after this fix.
And I didn't get why d3d is disabled for all Intel without possibility 
to switch it on?


--Semyon

On 09.09.2016 02:10, Philip Race wrote:

The following is just for testing right ? It should not be in this webrev
as part of what you propose to push ..
--
src/java.desktop/windows/native/libawt/java2d/d3d/D3DBadHardware.h
Print this page

@@ -49,13 +49,10 @@
 // all versions must fail ("there's no version of the driver that 
passes")

 #define NO_VERSION D_VERSION(0x, 0x, 0x, 0x)

 static const ADAPTER_INFO badHardware[] = {

-// All Intel Chips.
-{ 0x8086, ALL_DEVICEIDS, NO_VERSION, OS_ALL },
-
 // ATI Mobility Radeon X1600, X1400, X1450, X1300, X1350
 // Reason: workaround for 6613066, 6687166
 // X1300 (four sub ids)
 { 0x1002, 0x714A, D_VERSION(6,14,10,6706), OS_WINXP },
 { 0x1002, 0x714A, D_VERSION(7,14,10,0567), OS_VISTA },

---

-phil.

On 9/8/16, 4:06 PM, Semyon Sadetsky wrote:

I have reworked fix to not affect ATI and NVidia.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.05/

--Semyon


On 9/9/2016 12:20 AM, Semyon Sadetsky wrote:



On 08.09.2016 22:57, Philip Race wrote:


Can you provide something like a rationale for why these particular 
values might work ?

Otherwise this seems like a fix that can't be reviewed, only tested.
So that testing will be important. If you can be sure it passes
on ATI, Nvidia, and Intel then we can take it .. otherwise we 
should defer this.
I suppose those fudge factors are obtained experimentally. Not sure 
that any rationale is possible here.
The fix simply tested on different hardware. I hope after this fix 
D3D maybe enabled again for Intel APUs.

Currently it has been blacklisted in 8039444.

--Semyon


IIRC Semyon will need to change the code to bypass the check
for Intel hardware. There is no env. variable or system property to 
do this.


-phil.

On 9/8/16, 3:47 AM, Sergey Bylokhov wrote:

On 05.09.16 13:36, Semyon Sadetsky wrote:

At last I could get NVidia machine (special thanks to Yuri).

The updated fix should improve the situation on NVidia. For that one
common height/width fudge factor was separated in two different.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.04/


Can you please confirm that the fix and test works if d3d is 
enabled on the intel vk? I recall that d3d was disabled on intel 
so probably to check that we need to force d3d manually.



On 3/18/2016 9:12 AM, Semyon Sadetsky wrote:

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with fudge 
factors

values but I don't have nvidia based video to test.

--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having windows build
problems and
it passed on my ATI card.

-phil.


On 03/01/2016 04:37 PM, Sergey Bylokhov wrote:

On 15.01.16 9:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact
pixel to
pixel image but the scaled one. So Mac platform should be 
excluded

from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


I run the latest test(webrev.03) on my nvidia card, and it fails
after the fix, but pass before =(. I have no ati to check. Also I
cannot check the fix on intel card, because I cannot enable 
d3d on it.




--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments
seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified 
hardware-dependent

territory but
it seems quite possible that this could just as easily 
introduce a

different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to 
include at

least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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


The root cause is incorrect coordinate rounding in D3D 
renderer. To

fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

I have reworked fix to not affect ATI and NVidia.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.05/

--Semyon


On 9/9/2016 12:20 AM, Semyon Sadetsky wrote:



On 08.09.2016 22:57, Philip Race wrote:


Can you provide something like a rationale for why these particular 
values might work ?

Otherwise this seems like a fix that can't be reviewed, only tested.
So that testing will be important. If you can be sure it passes
on ATI, Nvidia, and Intel then we can take it .. otherwise we should 
defer this.
I suppose those fudge factors are obtained experimentally. Not sure 
that any rationale is possible here.
The fix simply tested on different hardware. I hope after this fix D3D 
maybe enabled again for Intel APUs.

Currently it has been blacklisted in 8039444.

--Semyon


IIRC Semyon will need to change the code to bypass the check
for Intel hardware. There is no env. variable or system property to 
do this.


-phil.

On 9/8/16, 3:47 AM, Sergey Bylokhov wrote:

On 05.09.16 13:36, Semyon Sadetsky wrote:

At last I could get NVidia machine (special thanks to Yuri).

The updated fix should improve the situation on NVidia. For that one
common height/width fudge factor was separated in two different.

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.04/


Can you please confirm that the fix and test works if d3d is enabled 
on the intel vk? I recall that d3d was disabled on intel so probably 
to check that we need to force d3d manually.



On 3/18/2016 9:12 AM, Semyon Sadetsky wrote:

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with fudge 
factors

values but I don't have nvidia based video to test.

--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having windows build
problems and
it passed on my ATI card.

-phil.


On 03/01/2016 04:37 PM, Sergey Bylokhov wrote:

On 15.01.16 9:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact
pixel to
pixel image but the scaled one. So Mac platform should be excluded
from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


I run the latest test(webrev.03) on my nvidia card, and it fails
after the fix, but pass before =(. I have no ati to check. Also I
cannot check the fix on intel card, because I cannot enable d3d 
on it.




--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments
seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified 
hardware-dependent

territory but
it seems quite possible that this could just as easily 
introduce a

different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to 
include at

least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D 
renderer. To

fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

At last I could get NVidia machine (special thanks to Yuri).

The updated fix should improve the situation on NVidia. For that one 
common height/width fudge factor was separated in two different.


http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.04/

--Semyon


On 3/18/2016 9:12 AM, Semyon Sadetsky wrote:

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with fudge factors 
values but I don't have nvidia based video to test.


--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having windows build 
problems and

it passed on my ATI card.

-phil.


On 03/01/2016 04:37 PM, Sergey Bylokhov wrote:

On 15.01.16 9:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact 
pixel to
pixel image but the scaled one. So Mac platform should be excluded 
from

testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


I run the latest test(webrev.03) on my nvidia card, and it fails 
after the fix, but pass before =(. I have no ati to check. Also I 
cannot check the fix on intel card, because I cannot enable d3d on it.




--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments 
seem to

indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified hardware-dependent
territory but
it seems quite possible that this could just as easily introduce a
different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to include at
least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D renderer. To
fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] [9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt

[Semyon Sadetsky]

On 7/7/2016 7:48 PM, Ajit Ghaisas wrote:


Thanks Phil for the review.
Please find my answers below.

Semyon, can you please comment on Phil's question below?

I agree with this change. The check is not needed.

--Semyon


Regards,
Ajit

-Original Message-
From: Phil Race
Sent: Wednesday, July 06, 2016 2:02 AM
To: Ajit Ghaisas
Cc: Sergey Bylokhov; Erik Joelsson; 2d-dev; awt-...@openjdk.java.net; 
build-...@openjdk.java.net
Subject: Re: [9] Fix for JDK-8074824: Resolve disabled warnings for libawt_xawt

It is not always clear to me what warning is being suppressed and why you have 
chosen a particular solution/action


this next one looks like it might introduce an unused variable warning.
What was it solving ? That the code was not checking a return value ?

size_t bytesWritten = write ( AWT_WRITEPIPE, _char, 1 ); //bytesWritten 
is unused

Isn't the compiler's point here that you *should* be doing something with the 
result?
Not just ignoring it differently ...

--
[Ajit] : there was a warning of type 'unused-result' for write() method. Now, I 
have just assigned that to a variable which fixes the warning.
I think, we should have some code to do error check on bytesWritten and return 
it - but it is out of purview of this fix as it may introduce behavioral change.
--

-

and this one ? I want Semyon to comment on what this code is trying to do in 
its original form since it was added for GTK3.

@@ -1989,11 +2029,7 @@
   static guint8 recode_color(gdouble channel)
   {
   guint16 result = (guint16)(channel * 65535);
-if (result < 0) {
-result = 0;
-} else if (result > 65535) {
-result = 65535;
-}
+
   return (guint8)( result >> 8);
   }

-
[Ajit] : there was a warning about guint16 will not be less than 0 and larger 
than 65535. Hence I have removed code checking this range.
-

-


-phil.

On 06/23/2016 12:09 AM, Ajit Ghaisas wrote:

Hi,

Bug :
  https://bugs.openjdk.java.net/browse/JDK-8074824
  (Resolve disabled warnings for libawt_xawt)

As part of fixing this bug, I have -

  1. Fixed warnings in source code after removing blanket warning 
suppressions from makefile.

  2. In case the warning fix is not possible, converted blanket warning 
suppression for this library to suppression of warnings for individual files.

  3. Added comments in makefile for the warning suppression that cannot be 
fixed.

 One type of gcc warning 'deprecated-declarations' will be fixed
separately (as part of JDK-8160146)


I have built the changes successfully on all supported platforms.


Webrev :
  http://cr.openjdk.java.net/~aghaisas/8074824/webrev.00/

Request you to review.

Regards,
Ajit

Re: [OpenJDK 2D-Dev] RFR: 8158408: Font2DTest demo needs to use FontPanel resolution matching the screen

[Semyon Sadetsky]

Looks good to me.

--Semyon


On 6/1/2016 9:44 PM, Phil Race wrote:

On 06/01/2016 11:05 AM, Semyon Sadetsky wrote:
you call  super.paintComponent(g); twice in lines 956 and 964. Is it 
expected behavior?


Yes, if "CannotDrawException" is caught we want to repaint using super 
else, we will

may have a partially panel.

-phil.



--Semyon


On 6/1/2016 7:58 PM, Philip Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8158408
Webrev: http://cr.openjdk.java.net/~prr/8158408/

At 125% scaling on JDK 9 Font2DTest looks horrid since its
BufferedImage is scaled to the screen.
Fix is to not use a BufferedImage, just use the Swing back buffer.

-phil.

Re: [OpenJDK 2D-Dev] RFR: 8158408: Font2DTest demo needs to use FontPanel resolution matching the screen

[Semyon Sadetsky]

you call  super.paintComponent(g); twice in lines 956 and 964. Is it 
expected behavior?


--Semyon


On 6/1/2016 7:58 PM, Philip Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8158408
Webrev: http://cr.openjdk.java.net/~prr/8158408/

At 125% scaling on JDK 9 Font2DTest looks horrid since its
BufferedImage is scaled to the screen.
Fix is to not use a BufferedImage, just use the Swing back buffer.

-phil.

Re: [OpenJDK 2D-Dev] RFR: 8146324: Add sun.font.FontUtilities.isComplexCharCode or related method to public API

[Semyon Sadetsky]

approved.

--Semyon

On 4/5/2016 9:45 PM, Phil Race wrote:
I have an approved CCC sitting waiting for a 2nd reviewer on the code 
change


-phil.

On 03/25/2016 11:31 AM, Sergey Bylokhov wrote:

Looks fine.

On 25.03.16 20:56, Phil Race wrote:

After a hallway conversation I have decided to change it to NPE.
Not that I think it matters greatly either way since I don't think
developers will be passing null to this method anyway :-)

The test is changed to match ..

http://cr.openjdk.java.net/~prr/8146324.1

-phil.

On 03/24/2016 03:59 PM, Sergey Bylokhov wrote:

On 25.03.16 1:22, Phil Race wrote:

That is true .. although I think I have previously been convinced
that IAE
is generally the better choice for such a case, I found only 
deriveFont

that throws IAE for null in this file.


deriveFont were changed(NPE to IAE) in 2001, but other methods and the
new code still use NPE. I think that in the most cases the null is not
taken into account and the specification is updated later(like the new
createFonts()).


So the question is whether to be consistent or to start picking the
better option.
So if there were no existing IAEs or NPEs in this file, which 
would you

choose now ?


Personally I always use NPE for null(via Objects.requireNonNull(...)).
and IAE for other incorrect arguments/types. or if the file(or the
same method) uses IAE already for null. As an example I use
System.getProperty(String).

But in Font.java we specify NPE for a few methods already, some other
methods throw NPE but w/o specification(except deriveFont...)



-phil.

On 03/24/2016 02:37 PM, Sergey Bylokhov wrote:

Hi, Phil.
Should this new method throw IAE? It seems that NPE can be thrown
instead, At least other methods in Font.java including the newly 
added

createFonts() throw NPE.

On 25.03.16 0:16, Phil Race wrote:

bug : https://bugs.openjdk.java.net/browse/JDK-8146324
fix : http://cr.openjdk.java.net/~prr/8146324/

Developers have used an internal API to detect when TextLayout or
GlyphVector.layoutGlyphVectorwill be needed since that is a slower
path.

This provides a public equivalent capability.

I have included a test which checks the assertions of the API.

A CCC will be filed.

-phil.

Re: [OpenJDK 2D-Dev] RFR 8149028: [TEST] add test for TIFFDirectory

[Semyon Sadetsky]

Looks good.

--Semyon

On 3/31/2016 6:52 PM, Alexander Stepanov wrote:

Please see
http://cr.openjdk.java.net/~avstepan/8149028/webrev.02/
(one more test covering TIFFImageReadParam was added).

Thanks,
Alexander

On 3/29/2016 8:21 PM, Alexander Stepanov wrote:

Please see the updated webrev:
http://cr.openjdk.java.net/~avstepan/8149028/webrev.01/
(one more test was added).

Thanks,
Alexander

On 3/25/2016 6:46 PM, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8149028/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8149028

Just a single test added.

Thanks,
Alexander

Re: [OpenJDK 2D-Dev] RFR: 8146324: Add sun.font.FontUtilities.isComplexCharCode or related method to public API

[Semyon Sadetsky]

On 3/25/2016 1:43 AM, Andrej Golovnin wrote:

Hi Phil,


That is true .. although I think I have previously been convinced that IAE
is generally the better choice for such a case, I found only deriveFont
that throws IAE for null in this file.
So the question is whether to be consistent or to start picking the better 
option.
So if there were no existing IAEs or NPEs in this file, which would you choose 
now ?

 From my standpoint of view you should use NPE.
And maybe java.util.Objects.requireNonNull() may help you to answer this 
question
if you rewrite your code to use it instead of doing manual check. :-)


requireNonNull() just checks null value regardless of where is it found. While 
the question is about about method argument values specifically.
I would be grateful to those who can prove why NPE is preferred to IAE in case 
if the invalid argument value equals to null.



Best regards,
Andrej Golovnin

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

Hi Phil,

Sergey wrote it fails on nvidia cards. I could play with fudge factors 
values but I don't have nvidia based video to test.


--Semyon

On 3/17/2016 11:05 PM, Phil Race wrote:

Semyon,

Any update on this ?
FWIW I used jprt to build your patch as I am having windows build 
problems and

it passed on my ATI card.

-phil.


On 03/01/2016 04:37 PM, Sergey Bylokhov wrote:

On 15.01.16 9:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact 
pixel to

pixel image but the scaled one. So Mac platform should be excluded from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


I run the latest test(webrev.03) on my nvidia card, and it fails 
after the fix, but pass before =(. I have no ati to check. Also I 
cannot check the fix on intel card, because I cannot enable d3d on it.




--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified hardware-dependent
territory but
it seems quite possible that this could just as easily introduce a
different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to include at
least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D renderer. To
fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

Please see the updated webrev 
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.03/.
The test is changed to execute per-pixel compare of on-screen and 
off-screen images in case of HiDPI device since the shape positioning 
depends on the scale factor.


--Semyon


On 2/7/2016 11:37 PM, Sergey Bylokhov wrote:

On 01.02.16 12:23, Semyon Sadetsky wrote:



On 1/21/2016 5:04 PM, Sergey Bylokhov wrote:

On 19/01/16 14:30, Semyon Sadetsky wrote:
We are getting error because of incorrect fudge factor added to the 
"to"

line coordinates (for line from - to).
See the updated version of the fix in which test should work on OS X:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.02/
Test fails on my mac book-pro with integrated i7 GPU - rectangle is
drawn at wrong position.


It will fail on any hdpi systems(including osx+retina), The reason is
that: when you create a VolatileImage it will take into account
default transform of the GraphicsConfiguration(on hdpi systems it will
be x2). When you draw a line to this VI and then take s spapshot then
you convert the big VI image to the small BI. you need to take into
account this scale and draw VI to BI 1-2-1, something like this:
BufferedImage capture = new BufferedImage(12x2, 12x2,
BufferedImage.TYPE_4BYTE_ABGR_PRE);
capture.createGraphics().drawImage(vi,0,0,12x2,12x2,null);

Note that even in this case the line locations check is incorrect and
should be adjusted.

I tried what you've proposed. The position of the up-scaled image is
wrong as well 1-21 instead of 2-22.


Why wrong? this is correct result, the simple BufferedImage will 
produce the same:
BufferedImage bi = new BufferedImage(12*2, 12*2, 
BufferedImage.TYPE_INT_ARGB_PRE);

Graphics2D g1 = bi.createGraphics();
g1.scale(2,2);
g1.setColor(Color.black);
g1.drawRect(1, 1, 9, 9);
g1.dispose();
ImageIO.write(bi,"png",new File("gold.png"));

When you draw a line you set the middle point of the line, not the 
left point. So the x=1px when scaled will cover the pixels 1 and 2, 
not the 2 and 3.



I think test may be remained the same.









--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments
seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified
hardware-dependent
territory but
it seems quite possible that this could just as easily
introduce a
different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to
include at
least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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


The root cause is incorrect coordinate rounding in D3D
renderer. To
fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 2/7/2016 11:37 PM, Sergey Bylokhov wrote:

On 01.02.16 12:23, Semyon Sadetsky wrote:



On 1/21/2016 5:04 PM, Sergey Bylokhov wrote:

On 19/01/16 14:30, Semyon Sadetsky wrote:
We are getting error because of incorrect fudge factor added to the 
"to"

line coordinates (for line from - to).
See the updated version of the fix in which test should work on OS X:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.02/
Test fails on my mac book-pro with integrated i7 GPU - rectangle is
drawn at wrong position.


It will fail on any hdpi systems(including osx+retina), The reason is
that: when you create a VolatileImage it will take into account
default transform of the GraphicsConfiguration(on hdpi systems it will
be x2). When you draw a line to this VI and then take s spapshot then
you convert the big VI image to the small BI. you need to take into
account this scale and draw VI to BI 1-2-1, something like this:
BufferedImage capture = new BufferedImage(12x2, 12x2,
BufferedImage.TYPE_4BYTE_ABGR_PRE);
capture.createGraphics().drawImage(vi,0,0,12x2,12x2,null);

Note that even in this case the line locations check is incorrect and
should be adjusted.

I tried what you've proposed. The position of the up-scaled image is
wrong as well 1-21 instead of 2-22.


Why wrong? this is correct result, the simple BufferedImage will 
produce the same:
BufferedImage bi = new BufferedImage(12*2, 12*2, 
BufferedImage.TYPE_INT_ARGB_PRE);

Graphics2D g1 = bi.createGraphics();
g1.scale(2,2);
g1.setColor(Color.black);
g1.drawRect(1, 1, 9, 9);
g1.dispose();
ImageIO.write(bi,"png",new File("gold.png"));

When you draw a line you set the middle point of the line, not the 
left point. So the x=1px when scaled will cover the pixels 1 and 2, 
not the 2 and 3.
No, the correct will be 2 and 3. In the small image 0 is white and 1 is 
black. In its 2x-upscaled variant 0 and 1 are white and 2 and 3 are black.
The resulting rectangle should be centered in the image but it is 
shifted to left top corner by 1px.



I think test may be remained the same.









--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments
seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified
hardware-dependent
territory but
it seems quite possible that this could just as easily
introduce a
different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to
include at
least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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


The root cause is incorrect coordinate rounding in D3D
renderer. To
fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] RFR: 8146881: [TEST] update some imageio plugins tests to affect TIFF format

[Semyon Sadetsky]

Looks good to me.

--Semyon

On 1/19/2016 7:17 PM, Alexander Stepanov wrote:

Hello Sergey,

please find the updated webrev:
http://cr.openjdk.java.net/~avstepan/8146881/webrev.01/index.html

Regards,
Alexander

On 1/18/2016 5:12 PM, Sergey Bylokhov wrote:

On 15/01/16 15:27, Alexander Stepanov wrote:

Hello Sergey,

 > Why this test do not cover the gif/bmp as well?
I can try to append the test, but it will fail for pair bmp +
TYPE_INT_ARGB due to
https://bugs.openjdk.java.net/browse/JDK-8147448


So we found a bug in jdk? I guess you can update the test to cover 
only working set of parameters and add a diff which should be made in 
the test to catch the problem.

This is why, I always suggest[1] to validate all supported formats(spi).

[1] 
http://mail.openjdk.java.net/pipermail/2d-dev/2016-January/006168.html




Regards,
Alexander

On 1/12/2016 5:49 PM, Sergey Bylokhov wrote:

Hi, Alexander.
BitDepth.java the @summary should be updated. Why this test do not
cover the gif/bmp as well?
It is unclear why biRGBATypes are unused in the test.

On 12/01/16 17:40, Alexander Stepanov wrote:

Please ignore mistaken changes in ImageWriterCompressionTest
(re-uploading the webrev...)

Thanks,
Alexander

On 1/12/2016 5:28 PM, Alexander Stepanov wrote:

Hello,

Some imageio plugins reg. tests should probably be updated to 
reflect

the fact that TIFF format is supported now.

Could you please let me know if the following changes are 
appropriate?

http://cr.openjdk.java.net/~avstepan/8146881/webrev.00/index.html
(bug: https://bugs.openjdk.java.net/browse/JDK-8146881)

Thanks,
Alexander

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 1/16/2016 12:10 AM, Sergey Bylokhov wrote:

On 15/01/16 19:40, Semyon Sadetsky wrote:



On 1/15/2016 6:46 PM, Sergey Bylokhov wrote:

On 15/01/16 17:29, Semyon Sadetsky wrote:

On 1/15/2016 4:30 PM, Sergey Bylokhov wrote:

On 15/01/16 09:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact
pixel to
pixel image but the scaled one. So Mac platform should be excluded
from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


In this cases non-retina hw will not be covered. If there are some
issues in the Robot, then you can skip it and use VolatileImage as a
destination for rendering.
But the issue is reproducible in on-screen painting on Windows 
platform.

It isn't necessary to spent extra efforts on the Mac workaround. The
test can be extended to the Mac platform later when Robot is fixed.


I am sure that the problem is not in the robot itself, but in the fact
the test does not take into account that device scale is not necessary
=1 in which case it calculates incorrect expected coordinates. If this
is the case then the same issue will be on win/linux with HiDPI 
display.

Not sure. I have Linux with retina display. The problem with Mac is that
lines are drawn with 1 pixel precision but screenshots are produced in
low resolution and 1px width black line becomes a blured gray line on
it. So is not possible to autotest with such inaccurate instrument.


This problem should go away if usage of robot class will be skipped, 
just draw to VolatileImage and check its pixels via getSnapshot()(take 
into account default device scale). 1pt line in case of retina should 
be drawn in 2px.



Can you also provide an additional information/examples on which
coordinates we get an rounding error.

Please look on the left screenshot attached to the JIRA. Checkbox
rectangles are wrong.


I mean the coordinates and code where we get this error. Something 
like this(when we draw the line in point 10 to point 12 we get an 
error when we calculate 12-10 etc)
We are getting error because of incorrect fudge factor added to the "to" 
line coordinates (for line from - to).
See the updated version of the fix in which test should work on OS X: 
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.02/
Test fails on my mac book-pro with integrated i7 GPU - rectangle is 
drawn at wrong position.


--Semyon








--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments
seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified 
hardware-dependent

territory but
it seems quite possible that this could just as easily introduce a
different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to 
include at

least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D 
renderer. To

fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] RFR: 8145776: [TEST] add a test checking multipage tiff creation

[Semyon Sadetsky]

Looks good.

--Semyon

On 1/11/2016 2:49 PM, Alexander Stepanov wrote:

Hello Semyon,

Thank you, that eliminates the question with the internal package. 
Probably Brian meant the same (but I just didn't understand this at 
first).


Please review the updated webrev:
http://cr.openjdk.java.net/~avstepan/8145776/webrev.02/

Thanks,
Alexander

On 1/11/2016 11:59 AM, Semyon Sadetsky wrote:

Hello Alexander,

Why don't use the reader method analogue for writer? Then the test 
could use the public API which is preferable.


Just a cosmetic note. Lines should be wrapped at 80 position.

--Semyon

On 12/29/2015 3:44 PM, Alexander Stepanov wrote:

Hello Brian,

Thank you for the notes, please see the updated webrev:
http://cr.openjdk.java.net/~yan/8145776/webrev.01/

1., 3. - fixed
WRT 2.: this import is necessary to use TIFFImageWriter, 
TIFFImageWriterSpi. As it follows from 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988, the 
majority of new classes (excepting tag sets) were added to 
com.sun.imageio.plugins.tiff. Do you mean these classes would be 
moved to javax/imageio soon? (please note also a @module tag added 
for compatibility with modular java).


Thanks,
Alexander

On 12/29/2015 2:12 AM, Brian Burkhalter wrote:

Hello Alexander,

A few comments.

1) Lines 53,59: By convention it is better to put constants in 
upper case, e.g., NUM_IMAGES and BLACK_SIZE.


2) Line 45, 102: The test references the internal package 
com.sun.imageio.plugins.tiff. In general it is better to use the 
public packages.


3) Line 69: If the test fails, it would be good to know the random 
seed. It would also be good to be able to set the seed when running 
the test. The approach taken in core libraries may be seen in 
test/java/math/BigInteger/BigIntegerTest.java. The 
jdk.testlibrary.RandomFactory class is used to create the Random 
instance and to obtain any seed provided via Java properties. This 
requires the test tags “@library /lib/testlibrary” and “@build 
jdk.testlibrary.*” and it is also good to add “@key randomness.”


Regards,

Brian

On Dec 25, 2015, at 5:10 AM, Alexander Stepanov 
<alexander.v.stepa...@oracle.com> wrote:



Could you please review the following fix
http://cr.openjdk.java.net/~yan/8145776/webrev.00/ 
<http://cr.openjdk.java.net/%7Eyan/8145776/webrev.00/>

for
https://bugs.openjdk.java.net/browse/JDK-8145776

Just a single test added checking the correctness of multi-page 
TIFF image creation.

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 1/15/2016 4:30 PM, Sergey Bylokhov wrote:

On 15/01/16 09:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact pixel to
pixel image but the scaled one. So Mac platform should be excluded from
testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


In this cases non-retina hw will not be covered. If there are some 
issues in the Robot, then you can skip it and use VolatileImage as a 
destination for rendering.
But the issue is reproducible in on-screen painting on Windows platform. 
It isn't necessary to spent extra efforts on the Mac workaround. The 
test can be extended to the Mac platform later when Robot is fixed.




--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments seem to
indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified hardware-dependent
territory but
it seems quite possible that this could just as easily introduce a
different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to include at
least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D renderer. To
fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

On 1/15/2016 6:46 PM, Sergey Bylokhov wrote:

On 15/01/16 17:29, Semyon Sadetsky wrote:

On 1/15/2016 4:30 PM, Sergey Bylokhov wrote:

On 15/01/16 09:59, Semyon Sadetsky wrote:

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact 
pixel to
pixel image but the scaled one. So Mac platform should be excluded 
from

testing:
http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/


In this cases non-retina hw will not be covered. If there are some
issues in the Robot, then you can skip it and use VolatileImage as a
destination for rendering.

But the issue is reproducible in on-screen painting on Windows platform.
It isn't necessary to spent extra efforts on the Mac workaround. The
test can be extended to the Mac platform later when Robot is fixed.


I am sure that the problem is not in the robot itself, but in the fact 
the test does not take into account that device scale is not necessary 
=1 in which case it calculates incorrect expected coordinates. If this 
is the case then the same issue will be on win/linux with HiDPI display.
Not sure. I have Linux with retina display. The problem with Mac is that 
lines are drawn with 1 pixel precision but screenshots are produced in 
low resolution and 1px width black line becomes a blured gray line on 
it. So is not possible to autotest with such inaccurate instrument.

Anyway, our target is Windows D3D, not Mac.
Can you also provide an additional information/examples on which 
coordinates we get an rounding error.
Please look on the left screenshot attached to the JIRA. Checkbox 
rectangles are wrong.






--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:

This fudge factor was last adjusted in
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments 
seem to

indicate that
there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified hardware-dependent
territory but
it seems quite possible that this could just as easily introduce a
different artifact
on some other hardware.

What exactly are you testing on ? And I think it needs to include at
least one Nvidia
and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D renderer. To
fix the issue one of fudge factors was adjusted.

Another issue mentioning in the JIRA ticket is taken out as a
separate bug.

--Semyon

[OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D renderer. To fix 
the issue one of fudge factors was adjusted.


Another issue mentioning in the JIRA ticket is taken out as a separate bug.

--Semyon

Re: [OpenJDK 2D-Dev] [9] Review Request: 8146042 Offscreen rendering is different from onscreen one

[Semyon Sadetsky]

Hi Phil & Sergey,

I have integrated Intel GPU i5 and cannot test other hardware.
On Mac's retina display the screen capture doesn't return exact pixel to 
pixel image but the scaled one. So Mac platform should be excluded from 
testing:

http://cr.openjdk.java.net/~ssadetsky/8146042/webrev.01/

--Semyon

On 1/14/2016 9:23 PM, Phil Race wrote:
This fudge factor was last adjusted in 
https://bugs.openjdk.java.net/browse/JDK-6597822
way back before the D3D pipeline was released and the comments seem to 
indicate that

there was a fair amount of testing on different hardware.

I don't know why this seems to be in un-specified hardware-dependent 
territory but
it seems quite possible that this could just as easily introduce a 
different artifact

on some other hardware.

What exactly are you testing on ? And I think it needs to include at 
least one Nvidia

and one AMD/ATI card.

-phil.

On 1/14/2016 10:09 AM, Semyon Sadetsky wrote:

Hello,

Please review the fix for jdk9.

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

The root cause is incorrect coordinate rounding in D3D renderer. To 
fix the issue one of fudge factors was adjusted.


Another issue mentioning in the JIRA ticket is taken out as a 
separate bug.


--Semyon

Re: [OpenJDK 2D-Dev] RFR: 8145776: [TEST] add a test checking multipage tiff creation

[Semyon Sadetsky]

Hello Alexander,

Why don't use the reader method analogue for writer? Then the test could 
use the public API which is preferable.


Just a cosmetic note. Lines should be wrapped at 80 position.

--Semyon

On 12/29/2015 3:44 PM, Alexander Stepanov wrote:

Hello Brian,

Thank you for the notes, please see the updated webrev:
http://cr.openjdk.java.net/~yan/8145776/webrev.01/

1., 3. - fixed
WRT 2.: this import is necessary to use TIFFImageWriter, 
TIFFImageWriterSpi. As it follows from 
http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988, the 
majority of new classes (excepting tag sets) were added to 
com.sun.imageio.plugins.tiff. Do you mean these classes would be moved 
to javax/imageio soon? (please note also a @module tag added for 
compatibility with modular java).


Thanks,
Alexander

On 12/29/2015 2:12 AM, Brian Burkhalter wrote:

Hello Alexander,

A few comments.

1) Lines 53,59: By convention it is better to put constants in upper 
case, e.g., NUM_IMAGES and BLACK_SIZE.


2) Line 45, 102: The test references the internal package 
com.sun.imageio.plugins.tiff. In general it is better to use the 
public packages.


3) Line 69: If the test fails, it would be good to know the random 
seed. It would also be good to be able to set the seed when running 
the test. The approach taken in core libraries may be seen in 
test/java/math/BigInteger/BigIntegerTest.java. The 
jdk.testlibrary.RandomFactory class is used to create the Random 
instance and to obtain any seed provided via Java properties. This 
requires the test tags “@library /lib/testlibrary” and “@build 
jdk.testlibrary.*” and it is also good to add “@key randomness.”


Regards,

Brian

On Dec 25, 2015, at 5:10 AM, Alexander Stepanov 
 wrote:



Could you please review the following fix
http://cr.openjdk.java.net/~yan/8145776/webrev.00/ 


for
https://bugs.openjdk.java.net/browse/JDK-8145776

Just a single test added checking the correctness of multi-page TIFF 
image creation.