Just to clarify one question, are you sure that this new calculation will make 
the correct rectangle to draw the text?

Currently this rectangle is created based on the size of the component, so this 
rectangle is smaller that the size.
But it looks like after the fix the rectangle will be bigger. Probably the bug 
exists in getPreferredSize() where we return small size?

Can you please confirm that this check will always work as expected:
176             if 
(!font.getTransform().equals(((Graphics2D)g).getTransform())) {
177                 AffineTransform tx = ((Graphics2D) g).getTransform();
178                 double scaleX = tx.getScaleX();
179                 double scaleY = tx.getScaleY();
180                 paintTextR.width = (int) Math.ceil(paintTextR.width * 
scaleX);
181                 paintTextR.height = (int) Math.ceil(paintTextR.height * 
scaleY);
182             }
183             v.paint(g, paintTextR);

In what coordinate space the final paintTextR will be? I assume that that the 
v.paint() expects coordinate in the users space.


On 14/04/2019 06:28, Jayathirth Rao wrote:
Thanks for the clarification changes are fine.

Regards,
Jay

On 12-Apr-2019, at 5:22 PM, Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com 
<mailto:prasanta.sadhuk...@oracle.com>> wrote:

HI Jay,


On 12-Apr-19 3:30 PM, Prasanta Sadhukhan wrote:

HI Jay,

Thanks for your review.
On 12-Apr-19 2:50 PM, Jayathirth Rao wrote:
Hi Prasanta,

I was going through the trail of changes related to this fix.

In JDK-8178025 changes are made to update the BasicTooTipUI renderer when there 
is “graphicsConfiguration” property is changed and also we have updated 
GlyphPainter1 to get proper FontMetrics if there is mismatch between Container 
FM and default FM. Apart from this we have modified code to return floating 
point width GlyphPainter1.getSpan().

After this under JDK-8191428 floating point width calculation of 
GlyphPainter1.getSpan() is reverted as it has caused a regression. Even after 
JDK-8191428 change, you have mentioned that JDK-8178025 regression test 
GetSpanHiDpiBug.java is not failing. Is it because font metrics update done in 
sync() method is enough to get proper span?

Yes, and also BasicHTML.updateRenderer() was called due to GC change in 
BasicLabelUI which cause html to recreate the text viacreateHTMLView() using 
proper span .
Also in JDK-8201552, we have modified the way in which we update renderer in 
propertyChange() using SwingU2.isScaleChanged() instead of firing event in all 
cases where “graphicsConfiguration” is the property change. Will this in anyway 
modify the previously expected behaviour?
I believe JDK-8201552 catches "graphicsConfiguration" property event in SwingU2.isScaleChanged() 
and updates if there is a change in "scale", which anyway is what "graphicsConfiguration" 
property was introduced for,
so I think it will not modify behavour prior to JDK-8201552 and after 
JDK-8178025 fix.

Overall it looks like we are getting different scale values from 
Component.getFont.getTransform() compared to getting scale values directly from 
GC. May be we are not updating font properties when there is GC change.

Not sure on this whether we should update the Component font's transform. As 
this issue is primarily for html text in tooltip (ie it does not occur if 
tooltip uses normal text) which means BasicHTML.updateRenderer()
has already updated the html text ('s span) due to GC change, so only thing we 
can do is to update the rectangle size to contain the updated text.
Also in the present change do we need to scale insets of the rectangle along 
with width and height?
I am updating the whole "paintTextR" rectangle needed to contain the tooltip 
text which already has insets calculated in it.

Regards
Prasanta

Thanks,
Jay

On 29-Mar-2019, at 3:07 PM, Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com 
<mailto:prasanta.sadhuk...@oracle.com>> wrote:

Hi All,

Please review a fix for an issue where it is seen that HTML tooltips aren't big 
enough to contain their contents on Windows HiDPI displays, there by the last 
word of the tooltip text are missing.

It is an aftereffect of JDK-8178025 
<https://bugs.openjdk.java.net/browse/JDK-8178025> where we update glyph 
fontMetrics if current metrics is different from Container's fontmetrics.
Even if the affinetransform or uiScale is modified, metrics is updated as 
FontMetrics would have changed because of the transform.
This caused the tooltip rectangle to be not able to contain the text which is 
drawn with modified metrics.
Proposed fix is to check if graphics transform has been modified because of 
hidpi scale, in which case, scale the tooltip rectangle accordingly.

Bug: https://bugs.openjdk.java.net/browse/JDK-8213535
webrev: http://cr.openjdk.java.net/~psadhukhan/8213535/webrev.0/

I was not able to fashion an automated test for this, so created a manual test.

Regards
Prasanta






--
Best regards, Sergey.

Reply via email to