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> 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. >>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >>
