<Swing Dev> [13] RFR JDK-8213535:Windows HiDPI html lightweight tooltips are truncated

[Prasanta Sadhukhan]

Hi Sergey,


On 15-Apr-19 8:37 AM, Sergey Bylokhov wrote:
> Just to clarify one question, are you sure that this new calculation 
> will make the correct rectangle to draw the text?
>
I think so.
> 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);
>
I have checked the regression tests for JToolTip with this fix without 
any issue.
> In what coordinate space the final paintTextR will be? I assume that 
> that the v.paint() expects coordinate in the users space.
>
Iguess the paintTextR will be in user space as BasicHTML.paint() says 
the rectangle as the region to render to.

Regards
Prasanta
>
> 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.sadhukhan at oracle.com 
>>> <mailto:prasanta.sadhukhan at 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.sadhukhan at oracle.com 
>>>>>> <mailto:prasanta.sadhukhan at 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
>>>>>
>>>>
>>>
>>
>
>