Thanks for clarification. +1 for http://cr.openjdk.java.net/~prr/8242004.1/ <http://cr.openjdk.java.net/%7Eprr/8242004.1/>
Regards, Jay > On 17-Apr-2020, at 4:35 AM, Philip Race <philip.r...@oracle.com> wrote: > > > > On 4/12/20, 11:03 PM, Jayathirth D v 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. > > Yes. > >> I think we are making this somewhat specified in case of drawXXXX() API’s by >> explicitly checking for non invertible transform. > > > I think you are missing my point. The exception was being thrown from the > layout code, not the rendering code. > > If you go back to the program you used earlier and change to use a normal Tx, > eg :- > Change > AffineTransform at = new AffineTransform(1f, 0.0f, -15, 0.0, -1, -30); > To > AffineTransform at = new AffineTransform(4f, 0.0f, 0, 4.0f, 0, 0); > > you'll notice it doesn't draw at 4X scale. Still at the same default scale. > > The checks on the rendering path are for when the same TX passed to TextLayout > is on the graphics as well (the common case) and are to avoid going into this > TextLayout hole. > >> >> 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? > > Because you then allow it to flow into this code that no longer throws the > exception but goes > to all the trouble to create the TextLayout, probably get an outline and then > hand it off to be drawn but > the shape isn't something that you can see, probably because it was clipped. > > So rather than relying on all that and hoping the checks are very useful. > > -phil >> >> 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/%7Eprr/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. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>> >>>> >>
