Typo : It's good that we have removed *unused* inverseTx code.
> On 13-Apr-2020, at 11:33 AM, Jayathirth D v <jayathirth....@oracle.com> wrote:
>
> Hi Phil,
>
> Thanks for the clarification.
> It's good that we have removed used inverseTx code.
>
> Since we don’t have inverseTx check now, do we need to explicitly return from
> drawXXXX() when we have non invertible transform.
> I think we are making this somewhat specified in case of drawXXXX() API’s by
> explicitly checking for non invertible transform.
>
> In latest change I made small change of not returning from drawXXXX() API’s
> but continue the flow and nothing gets drawn. Which is expected behaviour I
> think. Do we need to verify FontInfo.nonInvertibleTx in SunGraphics2D and use
> it in drawXXXX() API’s in latest change where we have removed inverseTx check?
>
> Thanks,
> Jay
>
>> On 11-Apr-2020, at 2:33 AM, Philip Race <philip.r...@oracle.com
>> <mailto:philip.r...@oracle.com>> wrote:
>>
>> You're right.
>> This is because I did not apply the non-invertible transform on the graphics
>> and do
>> what would be more normal which is to call Graphics2D.getFontRenderContext()
>> to
>> create the TextLayout so that it matched. The constructor FRC is for layout
>> not rendering.
>> So in other words unless the non-invertible transform is applied to the
>> graphics it doesn't prevent rendering.
>> In fact this made me looks how we use the inverse Tx and it turns out that
>> currently *nothing* uses it.
>> So I've updated the webrev to remove it entirely along with unused code.
>>
>> Now the test should cover both cases.
>> But in this case - default FRC used for rendering - we will get what you see.
>>
>> Updated webrev : http://cr.openjdk.java.net/~prr/8242004.1/
>> <http://cr.openjdk.java.net/~prr/8242004.1/>
>>
>> -phil.
>>
>> On 4/10/20, 8:41 AM, Jayathirth D v wrote:
>>>
>>> Hi Phil,
>>>
>>> I see your point of allowing queries on text layout without throwing
>>> exceptions.
>>>
>>> I was also under the impression that we should not see text getting drawn
>>> when we try to draw it using TextLayout with your change.
>>>
>>> For more clarification I am adding what I tested :
>>> I used code from your test case and tried drawing using TextLayout and
>>> drawString(). Without your change in both the cases we see
>>> NoninvertibleTransformException. After your change in case of
>>> TextLayout.draw() we are actually seeing the text but in case of
>>> drawString() text is not getting drawn.
>>>
>>> Verification test I used:
>>> import javax.swing.*;
>>> import java.awt.*;
>>> import java.awt.font.FontRenderContext;
>>> import java.awt.font.TextLayout;
>>> import java.awt.geom.AffineTransform;
>>>
>>> public class NonInvertibleTransformTextTest extends JPanel {
>>>
>>> public void paint(Graphics g) {
>>> Graphics2D g2 = (Graphics2D)g;
>>>
>>> AffineTransform at = new AffineTransform(1f, 0.0f, -15, 0.0, -1,
>>> -30);
>>>
>>> // First use case of drawing using TextLayout
>>> FontRenderContext frc = new FontRenderContext(at, false, false);
>>> Font font = new Font(Font.DIALOG, Font.PLAIN, 12);
>>> TextLayout tl = new TextLayout("ABC", font, frc);
>>> tl.draw(g2, 50, 50);
>>>
>>> // Second use case of drawing using drawString()
>>> //g2.setTransform(at);
>>> //g2.drawString("ABC", 50, 50);
>>> }
>>>
>>> public static void main(String[] args) {
>>> JFrame f = new JFrame();
>>> f.getContentPane().add(new NonInvertibleTransformTextTest());
>>> f.setSize(300, 200);
>>> f.setVisible(true);
>>> }
>>> }
>>> May be I am wrongly using TextLayout.draw() to check expected behaviour
>>> after the change.
>>> Please clarify.
>>>
>>> Thanks,
>>> Jay
>>>
>>>> On 10-Apr-2020, at 7:45 PM, Philip Race <philip.r...@oracle.com
>>>> <mailto:philip.r...@oracle.com>> wrote:
>>>>
>>>> Oh and if you do draw it, it still goes through the GV path so nothing
>>>> should draw there.
>>>>
>>>> This is what I meant by :
>>>> > Subsequent rendering of the TextLayoutwill be handled by the other
>>>> > checks being added.
>>>>
>>>> The shape returned might be not be null but I don't think you'll get more
>>>> than a line ..
>>>>
>>>> -phil.
>>>>
>>>> On 4/10/20, 12:57 AM, Philip Race wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 4/9/20, 10:26 PM, Jayathirth D v wrote:
>>>>>>
>>>>>> Hi Phil,
>>>>>>
>>>>>> I went through all use cases captured in test case (TextLayout,
>>>>>> drawXXXX).
>>>>>>
>>>>>> With updated change there is difference in behaviour between how we
>>>>>> interpret non-invertible transform between TextLayout.draw() and
>>>>>> drawXXXX() API’s.
>>>>>> In case of TextLayout.draw() we are overriding non-invertible transform
>>>>>> and allowing text rendering to happen, but in case of drawXXXX() we just
>>>>>> return and doesn’t allow text rendering to continue. Is it okay to have
>>>>>> this difference in behaviour?
>>>>>
>>>>> It becomes tricky.
>>>>> Do you have a suggestion ?
>>>>> Remember that the TextLayout is returned and does not have to be drawn,
>>>>> but could be
>>>>> by both drawing it directly or asking for the outline shape and rendering
>>>>> that.
>>>>> It can also be queried for the layout etc. There needs to be something
>>>>> returned that
>>>>> does not cause other problems. And patently there can't be apps that
>>>>> would care because
>>>>> today they can't get that far.
>>>>> And there's no defined behaviour in this case.
>>>>>
>>>>> So if you have specific code suggestions ..
>>>>>>
>>>>>> Also in test case its better if we continue to test all use cases and
>>>>>> then fail instead of failing at first instance and added test case needs
>>>>>> change in Copyright year from 2015 to 2020.
>>>>>
>>>>> oops.
>>>>>
>>>>> -phil.
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Jay
>>>>>>
>>>>>>> On 10-Apr-2020, at 7:53 AM, Philip Race <philip.r...@oracle.com
>>>>>>> <mailto:philip.r...@oracle.com>> wrote:
>>>>>>>
>>>>>>> D**n copy/paste, yes you correctly inferred the webrev is at
>>>>>>> <cr-url>/<my openjdk id>/<bugid> ie :
>>>>>>> http://cr.openjdk.java.net/~prr/8242004/
>>>>>>> <http://cr.openjdk.java.net/%7Eprr/8242004/>
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>>
>>>>>>> On 4/9/20, 7:00 PM, Jayathirth D v wrote:
>>>>>>>>
>>>>>>>> Hi Phil,
>>>>>>>>
>>>>>>>> Please share webrev link, you have added JBS link for webrev.
>>>>>>>> I went to path where you usually share webrev's and found
>>>>>>>> http://cr.openjdk.java.net/~prr/8242004/
>>>>>>>> <http://cr.openjdk.java.net/%7Eprr/8242004/>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Jay
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 10-Apr-2020, at 12:49 AM, Philip Race <philip.r...@oracle.com
>>>>>>>>> <mailto:philip.r...@oracle.com>> wrote:
>>>>>>>>>
>>>>>>>>> Any takers ?
>>>>>>>>>
>>>>>>>>> -phil
>>>>>>>>>
>>>>>>>>> On 4/3/20, 1:29 PM, Philip Race wrote:
>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8242004
>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8242004>
>>>>>>>>>> webrev: https://bugs.openjdk.java.net/browse/JDK-8242004
>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8242004>
>>>>>>>>>>
>>>>>>>>>> Several code paths can end up in the method shown in the bug report
>>>>>>>>>> with a non-invertible transform.
>>>>>>>>>>
>>>>>>>>>> As much as possible, we can prevent them reaching here by checking
>>>>>>>>>> in the rendering code.
>>>>>>>>>>
>>>>>>>>>> If we do get here, which should now be possible only when directly
>>>>>>>>>> creating
>>>>>>>>>> a TextLayout, we can use a default TX. Subsequent rendering of the
>>>>>>>>>> TextLayout
>>>>>>>>>> will be handled by the other checks being added.
>>>>>>>>>>
>>>>>>>>>> A regression test is provided which checks various APIs to make sure
>>>>>>>>>> no
>>>>>>>>>> exception is thrown.
>>>>>>>>>>
>>>>>>>>>> -phil.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>
>
