[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-swing-dev
Subject:    Re: <Swing Dev> [13] RFR JDK-8214702:Wrong text position for whitespaced string in printing Swing te
From:       Prasanta Sadhukhan <prasanta.sadhukhan () oracle ! com>
Date:       2019-04-19 7:59:23
Message-ID: fb37ff99-298d-e62f-e3b3-adf70ba08d2e () oracle ! com
[Download RAW message or body]

On 18-Apr-19 12:31 AM, Phil Race wrote:
> 
> 
> On 4/16/19 3:38 AM, Prasanta Sadhukhan wrote:
> > 
> > Hi Phil,
> > 
> > It seems screenWidth or "advance of the string at screen-resolution" 
> > is 533 whereas string advance calculated using printer fontmetrics is 
> > 503
> > 
> > Now, TextLine#getJustifiedLine() [called from 
> > TextLayout.getJustifiedLayout] again calculates "justifyAdvance" for 
> > TextLineComponent which comes out to be 503,then it calculates
> > the actual justification "delta" by subtracting justifyAdvance from 
> > screenWidth which is 533-503=30 and it then does 
> > TextJustifier.justify(delta)
> > which calculates the amount by which each side of each glyph should 
> > grow or shrink.
> > 
> > Then TextLine.getJustifiedLine() applies this value by calling 
> > TextLineComponent.applyJustificationDeltas() where it
> > handle whitespace by modifying advance but   handle everything else 
> > by modifying position before and after,
> > so "spaces" seem to grow in size resulting in shifting of text with 
> > whitespaces.
> 
> The spaces being used to add needed or absorb surplus space is what I 
> understood the current code to be doing,
> 
> However I am not sure what you mean by this :-
> "but   handle everything else by modifying position before and after,"
> 
> Code run through text layout will always have glyph positions 
> assigned, so I would suppose modifying the position
> is how the spaces were handled too .. and of course this means running 
> through all preceding glyphs to make
> it consistent .. and I thought it was only adjusting the spaces, so 
> what is "everything else"
It seems when TextLineComponent.applyJustificationDeltas() is called, 
ExtendedTextSourceLabel.applyJustificationDeltas() handles that
http://hg.openjdk.java.net/jdk/client/file/dc6c5c53669b/src/java.desktop/share/classes/sun/font/ExtendedTextSourceLabel.java#l1015
 where it handles "whitespace" a bit different (if block) from other 
glyph (else block) where advance is is not considered, which is also 
conveyed   in the comment @1011
> > 
> > Since it was mentioned in TextLayout.getJustifiedLayout() that "   
> > For best results, justificationWidth should not be too different from 
> > the current advance of the line"
> 
> So for "best" results don't try to adjust a string that's 3 words and 
> 80 pixels to a 500 pixel width.
> 
> > I am proposing a fix to conditionally call 
> > TextLayout.getJustifiedLayout() only if the difference between 
> > justificationWidth and advance with for printer is more than 10.
> > 
> 
> I had proposed investigating what happens if you simply don't use 
> justification when the text will fit.
> Maybe you are refining that but I am not sure about the details.
> 10 is one arbitrary number and is not proportional to the length - 
> which is what I would be
> thinking about first in succh an approach
> And I am not even sure you mean what you say.
> You say "more" than 10, yet your code implements "less" than 10.
It was a typo in mail.
Modified   webrev to not use justification if string width of text is 
within screenWidth
http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.4/

Regards
Prasanta
> And moreover its an absolute value you are using, which re-introduces 
> the clipping problem, doesn't it ?
> ie you are purely looking at the difference and it isn't what I has 
> proposed and I thought you were trying.
> 
> -phil
> 
> > webrev: http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.3/
> > 
> > Regards
> > Prasanta
> > On 26-Feb-19 12:30 PM, Philip Race wrote:
> > > 
> > > 
> > > On 2/25/19, 10:21 PM, Prasanta Sadhukhan wrote:
> > > > 
> > > > Thanks Phil for review. So, you are doubting it will regress swing 
> > > > printing tests. As you told earlier, I have ran the following 
> > > > regression test with this fix
> > > > 
> > > 
> > > It may not regress a test if the test is not being tested under the
> > > same conditions for which it is created but I am telling you for a
> > > fact that the fix is wrong. screenWidth should be "width on the screen"
> > > > 
> > > > (https://bugs.openjdk.java.net/browse/JDK-6488219 The bug above is 
> > > > covered by java/awt/print/PrinterJob/SwingUIText.java)
> > > > and I did not notice any regression. Any other test we have for 
> > > > swing printing that we can run?
> > > > 
> > > 
> > > No idea which tests will show this today but I know it is an issue.
> > > No way we can push this and then just wait for the complaints.
> > > 
> > > > > > Previously we were using DEFAULT_FRC to make it a screen width 
> > > > which except for maybe needing to be updated for hi-dpi screens is 
> > > > what we want.
> > > > This issue was there from jdk1.6. If it is for hidpi screen, we 
> > > > would have seen it from jdk9 onwards where we supported hidpi, no?
> > > 
> > > What I am saying here is that DEFAULT_FRC means "screen frc" and
> > > I think that should have been updated in 1.9 but was missed because
> > > it (hidpi for windows) was not a small or contained task.
> > > This is an ancilliary observation of something that should be looked
> > > at entirely independent of this bug.
> > > 
> > > -phil.
> > > 
> > > > 
> > > > Regards
> > > > Prasanta
> > > > On 26-Feb-19 3:25 AM, Phil Race wrote:
> > > > > The current fix confused me for a while as I could not see how it was
> > > > > at all different than the existing code, since I can't imagine 
> > > > > when we'd
> > > > > ever take the "else" branch here :
> > > > > 533 TextLayout layout;
> > > > > 534 if (!isFontRenderContextPrintCompatible(frc, deviceFRC)) {
> > > > > 535 layout = createTextLayout(c, text, g2d.getFont(), deviceFRC);
> > > > > 536 } else {
> > > > > 537 layout = createTextLayout(c, text, g2d.getFont(), frc);
> > > > > 538 } Eventually when walking through it I noticed this 531 
> > > > > FontMetrics fm = g2d.getFontMetrics();
> > > > > 532 float screenWidth = SwingUtilities2.stringWidth(c, fm 
> > > > > ,trimmedText);
> > > > > 
> > > > > "fm" from line 532 is getting a FontMetrics from the PRINTER - ie 
> > > > > the scaled FontRenderContext.
> > > > > It then uses this to calculate the advance width for such a case - 
> > > > > ie the printer
> > > > > but then *assigns it to a variable called screenWidth*.
> > > > > 
> > > > > Previously we were using DEFAULT_FRC to make it a screen width 
> > > > > which except
> > > > > for maybe needing to be updated for hi-dpi screens is what we want.
> > > > > 
> > > > > So in the updated proposed fix the wrong width is passed to   
> > > > > getJustifiedLayout().
> > > > > 
> > > > > This may not matter here because there is plenty of space, but in 
> > > > > other cases
> > > > > Swing printing will be clipped as a result. And there were many, 
> > > > > many, bug reports about
> > > > > that. Which is why the code is laying out to the screenwidth 
> > > > > because that is where the
> > > > > UI component size available came from. Buttons & Label text are 
> > > > > the typical cases where
> > > > > this showed up.
> > > > > 
> > > > > There maybe other things to change, as well but the incorrect 
> > > > > screenWidth is the
> > > > > main problem I see here.
> > > > > 
> > > > > -phil.
> > > > > 
> > > > > On 2/25/19 12:05 AM, Prasanta Sadhukhan wrote:
> > > > > > 
> > > > > > 
> > > > > > On 21-Feb-19 4:50 AM, Sergey Bylokhov wrote:
> > > > > > > On 13/02/2019 22:53, Prasanta Sadhukhan wrote:
> > > > > > > > Hi Sergey,
> > > > > > > > 
> > > > > > > > I believe drawChars() also has same printing issue [and should 
> > > > > > > > be changed like modified drawString()] but I am not able to 
> > > > > > > > test it as reproducer testcase uses JLabel whose constructor 
> > > > > > > > can only accept "String" and not char[] so I can only test 
> > > > > > > > drawString(). Using drawChars() implementation in drawString() 
> > > > > > > > still reproduces the issue.
> > > > > > > 
> > > > > > > Is it possible temporary replace the call to drawString() by the 
> > > > > > > drawChars(), to check how drawChars() will work?
> > > > > > As I told, it behaves similarly to unmodified drawString and the 
> > > > > > issue can still be seen. I think we should commit this 
> > > > > > drawString() change in this fix and I can open another bug to 
> > > > > > investigate drawChars() impl and reproducer. Will that be fine?
> > > > > > 
> > > > > > Regards
> > > > > > Prasanta
> > > > > > > or probably we can implement drawChars() on top of drawString()?
> > > > > > > 
> > > > > > > > 
> > > > > > > > Regards
> > > > > > > > Prasanta
> > > > > > > > On 14-Feb-19 4:12 AM, Sergey Bylokhov wrote:
> > > > > > > > > Hi, Prasanta.
> > > > > > > > > 
> > > > > > > > > > I modified the fix to use deviceFRC if not compatible and in 
> > > > > > > > > > sync with the comment which says "obtain a TextLayout with 
> > > > > > > > > > advances for the printer graphics FRC"
> > > > > > > > > > I used SwingUtilies2.getStringWidth() which calculates the 
> > > > > > > > > > advances of the string if text layouting is used.
> > > > > > > > > > 
> > > > > > > > > > http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.2/
> > > > > > > > > 
> > > > > > > > > Can you please take a look to the existed drawChars() method, 
> > > > > > > > > which is implemented in the similar way as drawStringImpl() 
> > > > > > > > > and new version of drawString(), but they have some small 
> > > > > > > > > difference. Why we cannot use the same logic?
> > > > > > > > > 
> > > > > > > > > For example in the drawChars:
> > > > > > > > > =========
> > > > > > > > > FontRenderContext deviceFontRenderContext = g2d.
> > > > > > > > > getFontRenderContext();
> > > > > > > > > FontRenderContext frc = getFontRenderContext(c);
> > > > > > > > > if (frc != null &&
> > > > > > > > > !isFontRenderContextPrintCompatible
> > > > > > > > > (deviceFontRenderContext, frc)) {
> > > > > > > > > String text = new String(data, offset, length);
> > > > > > > > > TextLayout layout = new TextLayout(text, 
> > > > > > > > > g2d.getFont(),
> > > > > > > > > deviceFontRenderContext);
> > > > > > > > > String trimmedText = trimTrailingSpaces(text);
> > > > > > > > > if (!trimmedText.isEmpty()) {
> > > > > > > > > float screenWidth = (float)g2d.getFont().
> > > > > > > > > getStringBounds(trimmedText, frc).getWidth();
> > > > > > > > > layout = 
> > > > > > > > > layout.getJustifiedLayout(screenWidth);
> > > > > > > > > 
> > > > > > > > > ==========
> > > > > > > > > Similar but not the same logic in the fix:
> > > > > > > > > 
> > > > > > > > > 524                                 FontRenderContext frc = 
> > > > > > > > > getFontRenderContext(c);
> > > > > > > > > 525                                 if (frc.isAntiAliased() || 
> > > > > > > > > frc.usesFractionalMetrics()) {
> > > > > > > > > 526                                         frc = new 
> > > > > > > > > FontRenderContext(frc.getTransform(), false, false);
> > > > > > > > > 527                                 }
> > > > > > > > > 528                                 FontRenderContext deviceFRC = 
> > > > > > > > > g2d.getFontRenderContext();
> > > > > > > > > 529                                 String trimmedText = 
> > > > > > > > > trimTrailingSpaces(text);
> > > > > > > > > 530                                 if (!trimmedText.isEmpty()) {
> > > > > > > > > 531                                         FontMetrics fm = \
> > > > > > > > > g2d.getFontMetrics(); 532                                         \
> > > > > > > > > float screenWidth =  SwingUtilities2.stringWidth(c, fm \
> > > > > > > > > ,trimmedText); 533                                         \
> > > > > > > > > TextLayout layout; 534                                         if 
> > > > > > > > > (!isFontRenderContextPrintCompatible(frc, deviceFRC)) {
> > > > > > > > > 535                                                 layout = \
> > > > > > > > > createTextLayout(c,  text, g2d.getFont(), deviceFRC);
> > > > > > > > > 536                                         } else {
> > > > > > > > > 537                                                 layout = \
> > > > > > > > > createTextLayout(c,  text, g2d.getFont(), frc);
> > > > > > > > > 538                                         }
> > > > > > > > > 540                                         layout = 
> > > > > > > > > layout.getJustifiedLayout(screenWidth);
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > 
> > > > 
> > 
> 


[Attachment #3 (text/html)]

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><br>
    </p>
    <br>
    <div class="moz-cite-prefix">On 18-Apr-19 12:31 AM, Phil Race wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:c8008749-1c24-f5aa-63e7-61b1c7dc496f@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      <br>
      <br>
      <div class="moz-cite-prefix">On 4/16/19 3:38 AM, Prasanta
        Sadhukhan wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:caa2ae1a-d997-c5a8-c964-b737ac477a6e@oracle.com">
        <meta http-equiv="Content-Type" content="text/html;
          charset=utf-8">
        <p>Hi Phil,<br>
        </p>
        It seems screenWidth or "advance of the string at
        screen-resolution" is 533 whereas string advance calculated
        using printer fontmetrics is 503 <br>
        <br>
        Now, TextLine#getJustifiedLine() [called from
        TextLayout.getJustifiedLayout] again calculates "justifyAdvance"
        for TextLineComponent which comes out to be 503,then it
        calculates <br>
        the actual justification "delta" by subtracting justifyAdvance
        from screenWidth which is 533-503=30 and it then does
        TextJustifier.justify(delta) <br>
        which calculates the amount by which each side of each glyph
        should grow or shrink. <br>
        <br>
        Then TextLine.getJustifiedLine() applies this value by calling
        TextLineComponent.applyJustificationDeltas() where it <br>
          handle whitespace by modifying advance but   handle everything
        else by modifying position before and after,<br>
          so "spaces" seem to grow in size resulting in shifting of text
        with whitespaces. <br>
      </blockquote>
      <br>
      The spaces being used to add needed or absorb surplus space is
      what I understood the current code to be doing,<br>
      <br>
      However I am not sure what you mean by this :-<br>
      "but   handle everything else by modifying position before and
      after,"<br>
      <br>
      Code run through text layout will always have glyph positions
      assigned, so I would suppose modifying the position<br>
      is how the spaces were handled too .. and of course this means
      running through all preceding glyphs to make<br>
      it consistent .. and I thought it was only adjusting the spaces,
      so what is "everything else"<br>
    </blockquote>
    It seems when TextLineComponent.applyJustificationDeltas() is
    called, ExtendedTextSourceLabel.applyJustificationDeltas() handles
    that<br>
<a class="moz-txt-link-freetext" \
href="http://hg.openjdk.java.net/jdk/client/file/dc6c5c53669b/src/java.desktop/share/c \
lasses/sun/font/ExtendedTextSourceLabel.java#l1015">http://hg.openjdk.java.net/jdk/cli \
ent/file/dc6c5c53669b/src/java.desktop/share/classes/sun/font/ExtendedTextSourceLabel.java#l1015</a><br>
  where it handles "whitespace" a bit different (if block) from other
    glyph (else block) where advance is is not considered, which is also
    conveyed   in the comment @1011<br>
    <blockquote type="cite"
      cite="mid:c8008749-1c24-f5aa-63e7-61b1c7dc496f@oracle.com">
      <blockquote type="cite"
        cite="mid:caa2ae1a-d997-c5a8-c964-b737ac477a6e@oracle.com"> <br>
          Since it was mentioned in TextLayout.getJustifiedLayout() that
        "   For best results, justificationWidth should not be too
        different from the current advance of the line"<br>
      </blockquote>
      <br>
      So for "best" results don't try to adjust a string that's 3 words
      and 80 pixels to a 500 pixel width.<br>
      <br>
      <blockquote type="cite"
        cite="mid:caa2ae1a-d997-c5a8-c964-b737ac477a6e@oracle.com"> I am
        proposing a fix to conditionally call
        TextLayout.getJustifiedLayout() only if the difference between
        justificationWidth and advance with for printer is more than 10.<br>
        <br>
      </blockquote>
      <br>
      I had proposed investigating what happens if you simply don't use
      justification when the text will fit.<br>
      Maybe you are refining that but I am not sure about the details.<br>
      10 is one arbitrary number and is not proportional to the length -
      which is what I would be<br>
      thinking about first in succh an approach<br>
      And I am not even sure you mean what you say.<br>
      You say "more" than 10, yet your code implements "less" than 10.<br>
    </blockquote>
    It was a typo in mail.<br>
    Modified   webrev to not use justification if string width of text is
    within screenWidth<br>
    <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.4/">http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.4/</a><br>
  <br>
    Regards<br>
    Prasanta<br>
    <blockquote type="cite"
      cite="mid:c8008749-1c24-f5aa-63e7-61b1c7dc496f@oracle.com"> And
      moreover its an absolute value you are using, which re-introduces
      the clipping problem, doesn't it ?<br>
      ie you are purely looking at the difference and it isn't what I
      has proposed and I thought you were trying.<br>
      <br>
      -phil<br>
      <br>
      <blockquote type="cite"
        cite="mid:caa2ae1a-d997-c5a8-c964-b737ac477a6e@oracle.com">
        webrev: <a class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Epsadhukhan/8214702/webrev.3/"
          moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.3/</a><br>
  <br>
        Regards<br>
        Prasanta<br>
        <div class="moz-cite-prefix">On 26-Feb-19 12:30 PM, Philip Race
          wrote:<br>
        </div>
        <blockquote type="cite" cite="mid:5C74E422.4030008@oracle.com">
          <meta content="text/html; charset=utf-8"
            http-equiv="Content-Type">
          <br>
          <br>
          On 2/25/19, 10:21 PM, Prasanta Sadhukhan wrote:
          <blockquote
            cite="mid:f089326e-ab71-84ca-eb62-b1cd16732bb7@oracle.com"
            type="cite">
            <meta http-equiv="Content-Type" content="text/html;
              charset=utf-8">
            <p>Thanks Phil for review. So, you are doubting it will
              regress swing printing tests. As you told earlier, I have
              ran the following regression test with this fix<br>
            </p>
          </blockquote>
          <br>
          It may not regress a test if the test is not being tested
          under the<br>
          same conditions for which it is created but I am telling you
          for a<br>
          fact that the fix is wrong. screenWidth should be "width on
          the screen"<br>
          <blockquote
            cite="mid:f089326e-ab71-84ca-eb62-b1cd16732bb7@oracle.com"
            type="cite">
            <p> (<a moz-do-not-send="true" class="moz-txt-link-freetext"
                href="https://bugs.openjdk.java.net/browse/JDK-6488219">https://bugs.openjdk.java.net/browse/JDK-6488219</a>
  The bug above is covered by
              java/awt/print/PrinterJob/SwingUIText.java)<br>
              and I did not notice any regression. Any other test we
              have for swing printing that we can run?<br>
            </p>
          </blockquote>
          <br>
          No idea which tests will show this today but I know it is an
          issue.<br>
          No way we can push this and then just wait for the complaints.<br>
          <br>
          <blockquote
            cite="mid:f089326e-ab71-84ca-eb62-b1cd16732bb7@oracle.com"
            type="cite">
            <p> </p>
            &gt;&gt;Previously we were using   <span class="changed">DEFAULT_FRC
              to make it a screen width which except for maybe needing
              to be updated for hi-dpi screens is what we want.<br>
              This issue was there from jdk1.6. If it is for hidpi
              screen, we would have seen it from jdk9 onwards where we
              supported hidpi, no?<br>
            </span></blockquote>
          <br>
          What I am saying here is that DEFAULT_FRC means "screen frc"
          and<br>
          I think that should have been updated in 1.9 but was missed
          because<br>
          it (hidpi for windows) was not a small or contained task.<br>
          This is an ancilliary observation of something that should be
          looked<br>
          at entirely independent of this bug.<br>
          <br>
          -phil.<br>
          <br>
          <blockquote
            cite="mid:f089326e-ab71-84ca-eb62-b1cd16732bb7@oracle.com"
            type="cite"><span class="changed"> <br>
              Regards<br>
              Prasanta<br>
            </span>
            <div class="moz-cite-prefix">On 26-Feb-19 3:25 AM, Phil Race
              wrote:<br>
            </div>
            <blockquote type="cite"
              cite="mid:9a1ceec7-d4e7-de3b-0033-90689095278a@oracle.com">
              <meta http-equiv="Content-Type" content="text/html;
                charset=utf-8">
              The current fix confused me for a while as I could not see
              how it was<br>
              at all different than the existing code, since I can't
              imagine when we'd<br>
              ever take the "else" branch here :<br>
              <pre><span class="changed"> 533                     TextLayout \
layout;</span> <span class="changed"> 534                     if \
(!isFontRenderContextPrintCompatible(frc, deviceFRC)) {</span> <span class="changed"> \
535                         layout = createTextLayout(c, text, g2d.getFont(), \
deviceFRC);</span> <span class="changed"> 536                     } else {</span>
<span class="changed"> 537                         layout = createTextLayout(c, text, \
g2d.getFont(), frc);</span> <span class="changed"> 538                     }

Eventually when walking through it I noticed this

</span><span class="changed"> 531                     FontMetrics fm = \
g2d.getFontMetrics();</span> <span class="changed"> 532                     float \
screenWidth = SwingUtilities2.stringWidth(c, fm ,trimmedText);</span> <span \
class="changed"></span></pre>  <div class="moz-cite-prefix"><br>
                "fm" from line 532 is getting a FontMetrics from the
                PRINTER - ie the scaled FontRenderContext.<br>
                It then uses this to calculate the advance width for
                such a case - ie the printer<br>
                but then *assigns it to a variable called screenWidth*.<br>
                <br>
                Previously we were using   <span class="changed">DEFAULT_FRC
                  to make it a screen width which except<br>
                  for maybe needing to be updated for hi-dpi screens is
                  what we want.<br>
                  <br>
                </span>So in the updated proposed fix the wrong width is
                passed to   getJustifiedLayout().<br>
                <br>
                This may not matter here because there is plenty of
                space, but in other cases<br>
                Swing printing will be clipped as a result. And there
                were many, many, bug reports about<br>
                that. Which is why the code is laying out to the
                screenwidth because that is where the<br>
                UI component size available came from. Buttons &amp;
                Label text are the typical cases where<br>
                this showed up.<br>
                <br>
                There maybe other things to change, as well but the
                incorrect screenWidth is the<br>
                main problem I see here.<br>
                <br>
                -phil.<br>
                <br>
                On 2/25/19 12:05 AM, Prasanta Sadhukhan wrote:<br>
              </div>
              <blockquote type="cite"
                cite="mid:8b3e423f-55ef-63a6-eca1-481a8e29be4c@oracle.com">
                <br>
                <br>
                On 21-Feb-19 4:50 AM, Sergey Bylokhov wrote: <br>
                <blockquote type="cite">On 13/02/2019 22:53, Prasanta
                  Sadhukhan wrote: <br>
                  <blockquote type="cite">Hi Sergey, <br>
                    <br>
                    I believe drawChars() also has same printing issue
                    [and should be changed like modified drawString()]
                    but I am not able to test it as reproducer testcase
                    uses JLabel whose constructor can only accept
                    "String" and not char[] so I can only test
                    drawString(). Using drawChars() implementation in
                    drawString() still reproduces the issue. <br>
                  </blockquote>
                  <br>
                  Is it possible temporary replace the call to
                  drawString() by the drawChars(), to check how
                  drawChars() will work? <br>
                </blockquote>
                As I told, it behaves similarly to unmodified drawString
                and the issue can still be seen. I think we should
                commit this drawString() change in this fix and I can
                open another bug to investigate drawChars() impl and
                reproducer. Will that be fine? <br>
                <br>
                Regards <br>
                Prasanta <br>
                <blockquote type="cite">or probably we can implement
                  drawChars() on top of drawString()? <br>
                  <br>
                  <blockquote type="cite"> <br>
                    Regards <br>
                    Prasanta <br>
                    On 14-Feb-19 4:12 AM, Sergey Bylokhov wrote: <br>
                    <blockquote type="cite">Hi, Prasanta. <br>
                      <br>
                      <blockquote type="cite">I modified the fix to use
                        deviceFRC if not compatible and in sync with the
                        comment which says "obtain a TextLayout with
                        advances for the printer graphics FRC" <br>
                        I used SwingUtilies2.getStringWidth() which
                        calculates the advances of the string if text
                        layouting is used. <br>
                        <br>
                        <a class="moz-txt-link-freetext"
                          \
                href="http://cr.openjdk.java.net/%7Epsadhukhan/8214702/webrev.2/"
                          \
moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/8214702/webrev.2/</a>  \
<br>  </blockquote>
                      <br>
                      Can you please take a look to the existed
                      drawChars() method, which is implemented in the
                      similar way as drawStringImpl() and new version of
                      drawString(), but they have some small difference.
                      Why we cannot use the same logic? <br>
                      <br>
                      For example in the drawChars: <br>
                      ========= <br>
                                             FontRenderContext
                      deviceFontRenderContext = g2d. <br>
                                                     getFontRenderContext(); <br>
                                             FontRenderContext frc =
                      getFontRenderContext(c); <br>
                                             if (frc != null &amp;&amp; <br>
                                                    
                      !isFontRenderContextPrintCompatible <br>
                                                     (deviceFontRenderContext, frc)) \
{  <br>
                                                       String text = new String(data,
                      offset, length); <br>
                                                       TextLayout layout = new
                      TextLayout(text, g2d.getFont(), <br>
                                                                                  
                      deviceFontRenderContext); <br>
                                                       String trimmedText =
                      trimTrailingSpaces(text); <br>
                                                       if (!trimmedText.isEmpty()) { \
                <br>
                                                               float screenWidth =
                      (float)g2d.getFont(). <br>
                                                                      
                      getStringBounds(trimmedText, frc).getWidth(); <br>
                                                               layout =
                      layout.getJustifiedLayout(screenWidth); <br>
                      <br>
                      ========== <br>
                      Similar but not the same logic in the fix: <br>
                      <br>
                        524                                 FontRenderContext frc =
                      getFontRenderContext(c); <br>
                        525                                 if (frc.isAntiAliased() \
||  frc.usesFractionalMetrics()) { <br>
                        526                                         frc = new
                      FontRenderContext(frc.getTransform(), false,
                      false); <br>
                        527                                 } <br>
                        528                                 FontRenderContext \
deviceFRC =  g2d.getFontRenderContext(); <br>
                        529                                 String trimmedText =
                      trimTrailingSpaces(text); <br>
                        530                                 if \
(!trimmedText.isEmpty()) {  <br>
                        531                                         FontMetrics fm =
                      g2d.getFontMetrics(); <br>
                        532                                         float screenWidth \
                =
                      SwingUtilities2.stringWidth(c, fm ,trimmedText); <br>
                        533                                         TextLayout \
layout; <br>  534                                         if
                      (!isFontRenderContextPrintCompatible(frc,
                      deviceFRC)) { <br>
                        535                                                 layout =
                      createTextLayout(c, text, g2d.getFont(),
                      deviceFRC); <br>
                        536                                         } else { <br>
                        537                                                 layout =
                      createTextLayout(c, text, g2d.getFont(), frc); <br>
                        538                                         } <br>
                        540                                         layout =
                      layout.getJustifiedLayout(screenWidth); <br>
                      <br>
                      <br>
                      <br>
                    </blockquote>
                    <br>
                  </blockquote>
                  <br>
                  <br>
                </blockquote>
                <br>
              </blockquote>
              <br>
            </blockquote>
            <br>
          </blockquote>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic