FWD » Core Development » User Interface

I didn't implement this function. Please explain the reasons that in the code below the text x-position coordinate is reduced by a pixel

CanvasRenderer.prototype.drawText = function(text, x, y, centered)
{
   if (centered)
   {
      // in this case, the "y" is the height in which it needs to be vertically centered
      this.ctx.textBaseline = 'middle';
      ++y;
   }
   else
   {
      this.ctx.textBaseline = 'alphabetic'; // matches the Swing's baseline
      --y;
   }
   --x;

   var cfont = p2j.screen.getCurrentFont();
   /*start debug*/
   //   var txtHeight = p2j.fonts.getTextHeight(cfont, text);
   //   var textWidth = p2j.fonts.getTextWidth(cfont, text);
   //   
   //   if (centered)
   //   {
   //      this.drawRect(x, y - txtHeight / 2, textWidth, txtHeight, new Array(255, 255, 0), false);
   //   }
   //   else 
   //   {
   //      this.drawRect(x, y - 3 * txtHeight / 4, textWidth, txtHeight, new Array(0, 255, 0), false);
   //   }
   /*end debug*/

   this.ctx.fillText(text, x, y);

   if (p2j.fonts.isUnderlined(cfont))
   {
      // if the style is underline, draw a line under the text
      var txtHeight = p2j.fonts.getTextHeight(cfont, text);
      var textWidth = p2j.fonts.getTextWidth(cfont, text);

      var x1 = x;
      var x2 = x + textWidth;
      if (centered)
      {
         y = y + Math.round(txtHeight / 2) - Math.round(txtHeight / 6) + 1;
      }

      this.strokeLineSegment(x1, y, x2, y, this.rawColor);
   }
}

Committed revision 11280(3394a) should fix the web text position along a horizontal. It seems that the other text position issues are related to the common code (browser). Should I work on these browser issues?

Should I work on these browser issues?

Yes, but make sure you get input from Stanislav so that you don't cause regressions. He is our browse guru.

OK. It seems that table labels are displayed 1-pixel left due to this code ClassicTheme.this.drawBrowseCellText

   protected void drawBrowseCellText(BrowseColumnGuiImpl column,
                                     GuiDriver gd,
                                     boolean label,
                                     boolean selectedRow,
                                     String text,
                                     NativeRectangle clipRectangle,
                                     int lineNumber,
                                     int viewportRowIndex)
   {
      setBrowseCellForegroundColor(column, gd, label, selectedRow, viewportRowIndex);

      FontDetails fd = label ? column.getLabelFontDetails() : column.getColumnFontDetails();
      gd.setGuiFont(fd);

      int y = label ? column.getLabelTextTopOffset() : column.getCellTextTopOffset();
      y += fd.legacyHeight * lineNumber;

      int x = 0;
      if (Browse.isRightAligned(column))
      {
         x = clipRectangle.width() - column.getTextWidth(text,
               label ? column.getLabelFontNum() : column.getColumnFontNum());
      }
      if (label && !column.isLastColumn())
      {
         --x; // <--------------------- reducing x-coordinate ?
      }

      gd.drawString(text, x, y);
   }

It seems that table labels are displayed 1-pixel left due to this code ClassicTheme.this.drawBrowseCellText

This was added in the very first version of ClassicTheme by Ovidiu, so I cannot comment. As to me, just fix it and check positioning and clipping for swing/web, different themes, first/last/middle/last locked columns, label/cell text, left/right aligned text.

I cannot remember why I added that special processing. Most likely I adjusted that based on offset revealed when comparing the two layers in Gimp.
It is possible that the font configuration to had been different. If it is wrong, please drop it.

Ovidiu Maxiniuc wrote:

It is possible that the font configuration to had been different. If it is wrong, please drop it.

Ovidiu, could you help with correct font setup? It is supposed that text-metrics.xml is filled with all strings with their metrics. I have text-metrics.xml of 59 Mb dated 29.11.2017.

The text-metrics.xml for AW was last updated by Constantin on Tue 2017-11-28 22:20:15 +0200 as revno 110.
I don't know if I made myself very clear. When I said font configuration I meant the fonts actually used by FWD. For example, I don't recall if I was using MS Sans Serif or Microsoft Sans Serif (see #2765). They look similar but might be slightly different here and there. Probably I was using the unlicensed one at that time. So I suggested to drop my code that translate the text 1 pixel to the left in case the correct font gets cut because of it.

Thank you, if we don't shift the text by 1 pixel to the left, then some table headers don't fit the column and some right aligned column headers are not fitted the column width too. Planning to check the formula and the column widths with the original ones in these cases but at this moment I don't have a solution.

Please review committed revision 11296 that should fix drawing of labels with help of #3394-88.

Please review committed revision 11297.

Code Review Task Branch 3394a Revisions 11296/7

I don't see a specific issue with the changes. However, the validity of the changes to drawBrowseCellText() are not intuitively obvious to me.

Does the general removal of the --x for the label case avoid the left justified labels being truncated?

The cell text still seems too close the left side of the cell. There seems to be no spacing, but I think there is at least a pixel of spacing in the 4GL. Am I mistaken?

Stanislav/Hynek: please review.

Greg Shah wrote:

Stanislav/Hynek: please review.

Formally, I don't see anything wrong with the code. Maybe I would change the following

x -= (BrowseColumnGuiImpl.LABEL_OFFSET_RIGHT + BrowseColumnGuiImpl.LABEL_OFFSET_LEFT);
int t = x - 1;
x = (t > 0) ? t : 0;

To something less cryptic:

x -= (BrowseColumnGuiImpl.LABEL_OFFSET_RIGHT + BrowseColumnGuiImpl.LABEL_OFFSET_LEFT + 1);
if (x < 0)
{
   x = 0;
}

But I can't say whether the code is functionally correct.

Yes. Agree, committed rev 11301.

If Stanislav is OK with these changes, then we are good to go.

I'm not good, wait a bit.

Stanislav Lomany wrote:

It is not clear if actual P2J version (rev 11295) has issues with clipping (please use fonts without anti-aliasing for testing), however it is clear that rev 11297 has. Also note that rev 11297 has wrong column widths.

Stanislav, please describe font settings thoroughly, what are fonts without anti-aliasing? Is it true that P2J rev 11295 has correct column widths for the test you provided?
Did you run your test and make a screen shot for the Web client or the Swing client?

Did you build a font-metrics file for the strings from your test?

The rev.11297, 11301 don't make changes for column widths. It is supposed that they are calculated correctly.

Stanislav, please describe font settings thoroughly, what are fonts without anti-aliasing?

I don't know, but the browse title ("Static browse") has one without anti-aliasing.

Is it true that P2J rev 11295 has correct column widths for the test you provided?

They match on the screenshot.

Did you run your test and make a screen shot for the Web client or the Swing client?

Swing.

Did you build a font-metrics file for the strings from your test?

No.

The rev.11297, 11301 don't make changes for column widths.

You've changed the constants LABEL_OFFSET_LEFT and LABEL_OFFSET_RIGHT used by doLayout() -> getColumnPadding()

Stanislav Lomany wrote:

Stanislav, please describe font settings thoroughly, what are fonts without anti-aliasing?

I don't know, but the browse title ("Static browse") has one without anti-aliasing.

The problem is that I don't understand it too, otherwise this issue will be fixed.
Greg, I have no ideas how to proceed and I would like to recall that the same font unresolved issue was considered in the last iteration when Hotel GUI had been developed.

Is it true that P2J rev 11295 has correct column widths for the test you provided?

They match on the screenshot.

The rev.11297, 11301 don't make changes for column widths.

You've changed the constants LABEL_OFFSET_LEFT and LABEL_OFFSET_RIGHT used by doLayout() -> getColumnPadding()

Do these changes affect column widths?

Is this code from BrowseGuiImpl.doLayout() correct?

      // Update column.WIDTH attribute.
      int colNo = getColumnCount();
      for (int i = 0; i < colNo; i++)
      {
         BrowseColumnGuiImpl column = getColumn(i);

         int width = column.physicalDimension().width;
         width -= column.getColumnPadding(); // <----------------------------------------why the width is reduced?
         column.config().widthChars = cc.widthFromNative(width);
         column.config().widthPixels = width;
      }

Stanislav: Please take this task. Please remove the changes for 11296/11297 such that it won't cause regressions. If you would do this quickly, we are trying to get 3394a into the trunk.

Constantin: It seems this is important to put into 3394a ASAP.

Sergey: You can move to other tasks.

Greg Shah wrote:

Stanislav: Please take this task. Please remove the changes for 11296/11297 such that it won't cause regressions. If you would do this quickly, we are trying to get 3394a into the trunk.
Constantin: It seems this is important to put into 3394a ASAP.

I'm OK with this.

Stanislav: Please take this task. Please remove the changes for 11296/11297 such that it won't cause regressions. If you would do this quickly, we are trying to get 3394a into the trunk.

Do you want me to revert 11296/11297 and then you merge 3394 and I check into another branch OR you want my to try to fix this issue in the limited amount of time?

Do you want me to revert 11296/11297 and then you merge 3394 and I check into another branch

This one. The fix should be done in a separate branch.

Stanislav Lomany wrote:

Stanislav: Please take this task. Please remove the changes for 11296/11297 such that it won't cause regressions. If you would do this quickly, we are trying to get 3394a into the trunk.

Do you want me to revert 11296/11297 and then you merge 3394 and I check into another branch OR you want my to try to fix this issue in the limited amount of time?

After rebase the revisions in 3394a are 11299/11300 - although please make sure these are the correct revisions before rolling back the changes.

So, rollback these two revisions in 3394a and commit them. After that, you can consider 3394a frozen again - I'm waiting for runtime testing to complete, do some manual testing in the large GUI app and after that merge it to trunk.

So, rollback these two revisions in 3394a and commit them.

Done.

FYI.
To draw a columns separator we should decide to which column it belongs. In original design a column contained its left separator, however when we switched to UI themes, a column began to contain its right separator instead.
There is 4GL quirk when then right border of the locked panel is not fully drawn and you can see 1px of the underlying scrolling panel which makes me think that the original design was correct (but technically it does not state it 100%). Other that this case I'm not aware of other cases which tell which design is correct, so I'm leaving everything as is.

What is the status of this task?

Just finished, going to update the branch and check for regressions in AW.

The fix is committed into the branch 3435a rev 11256.

Code Review Task Branch 3435a Revision 11256

I'm fine with the changes.