Project

General

Profile

Bug #2857

Bug #2677: fix drawing and functional differences between P2J GUI and 4GL GUI

font differences for toggle box and menu

Added by Vadim Gindin over 5 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Vadim Gindin
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:

hidden_menu_elems.png (12.9 KB) Vadim Gindin, 11/18/2015 05:37 PM

fonts_compare.png (20.9 KB) Vadim Gindin, 11/18/2015 05:37 PM

tbx_label_positions.png (29.4 KB) Vadim Gindin, 01/07/2016 06:46 AM

menu_p2j_web_over_swing.png (6.99 KB) Vadim Gindin, 01/07/2016 07:06 AM

menu_p2j_web_over_windev01.png (5.14 KB) Vadim Gindin, 01/07/2016 07:06 AM

sm_compare2.png (10 KB) Vadim Gindin, 04/11/2016 07:00 AM

overritten_mi.png (1.43 KB) Vadim Gindin, 05/13/2016 05:19 AM

overritten_mi1.png (1.48 KB) Vadim Gindin, 05/13/2016 08:16 AM


Related issues

Related to User Interface - Bug #3134: CHUI menu issues when some labels are empty New 06/24/2016

History

#1 Updated by Vadim Gindin over 5 years ago

  1. Label position in toggle-box is different: higher than usual. It probably depends on font difference.
  2. (Menu) Labels are drawn highter than usual. It probably depends on font difference.
  3. Right after start and MENUBAR is drawn, when I move mouse pointer under the MENUBAR all items titles are drawn over workspace background as if corresponding sub-menus were opened.
  4. Sub-menu arrow in a title is drawn differently: as without left vertical line

#2 Updated by Vadim Gindin over 5 years ago

Here are the images
  1. Fonts compare of PROGRESS, P2J SWING, P2J WEB. There is an other font for WEB.
  2. MENU bug of showing hidden menu items

#3 Updated by Greg Shah over 5 years ago

  • Subject changed from Menu bugs in a web client. to toggle-box and menu bugs in a web client
  • Target version set to Milestone 12
  • Start date deleted (11/18/2015)

#4 Updated by Vadim Gindin over 5 years ago

Vadim Gindin wrote:

3 Right after start and MENUBAR is drawn, when I move mouse pointer under the MENUBAR all items titles are drawn over workspace background as if corresponding sub-menus were opened

This issue is already fixed

4 Sub-menu arrow in a title is drawn differently: as without left vertical line

Created task #2916

#5 Updated by Greg Shah over 5 years ago

  • Subject changed from toggle-box and menu bugs in a web client to font differences for toggle box and menu

#6 Updated by Vadim Gindin over 5 years ago

It's a tbx_present.p procedure. There are 2 fonts are used: tahoma, 8, true, false, false and ms sans serif, 8, false, false, false. The first one is used for Frame and WindowTitleBar. The second font is used for ToggleBox, MessageArea and others.

All attributes of these fonts are match (correspondingly tahoma-tahoma, sans-sans) between SWING and WEB client, except fontName for "Tahoma": fontName="Tahoma Bold" for SWING and fontName="Tahoma" for WEB.

What I see?
  1. I see the difference of drawing of the letters "G" and "g" in a frame titles and window titles (Tahoma font). Do you agree?
  2. I see some unclear difference in drawing toggle-box labels (Sans font) like drawing of a letter "k". It looks like Web client uses some font smoothing and SWING client does not use smoothing. Is that possible?

Do you see some other visual differences in attached image between SWING and WEB clients (second and third images from right to left)?

#7 Updated by Greg Shah over 5 years ago

This task is only about font/text positioning/text sizing differences for toggle box and menu. As you reported in note 1:

  1. Label position in toggle-box is different: higher than usual. It probably depends on font difference.
  2. (Menu) Labels are drawn highter than usual. It probably depends on font difference.

The toggle box screen captures in note 2 do seem to have label positioning issues (text is too high in P2J).

I don't see any menu screen captures that can show a comparison between Progress, the P2J swing client and the P2J web client. Please create and post them so we can assess the label positioning issues for menu.

Please focus on these issues only.

I see the difference of drawing of the letters "G" and "g" in a frame titles and window titles (Tahoma font). Do you agree?

Don't worry about this, we are working window/frame font issues separately.

I see some unclear difference in drawing toggle-box labels (Sans font) like drawing of a letter "k". It looks like Web client uses some font smoothing and SWING client does not use smoothing. Is that possible?

Yes, the web client uses anti-aliasing and the results are smoothed. Again, we are working that separately. Your task here is only to resolve the text positioning/text sizing differences in regard to toggle box and menus.

#8 Updated by Vadim Gindin over 5 years ago

Greg Shah wrote:

This task is only about font/text positioning/text sizing differences for toggle box and menu. As you reported in note 1:

  1. Label position in toggle-box is different: higher than usual. It probably depends on font difference.
  2. (Menu) Labels are drawn highter than usual. It probably depends on font difference.

The toggle box screen captures in note 2 do seem to have label positioning issues (text is too high in P2J).
..

Have a look at the following compare of label position for WINDEV01 and P2J WEB client: I've added Ave piece of WINDEV01 screenshot over P2J WEB client screenshot superposing checkboxes and highlight rectangles.

At the same time P2J SWING client and WINDEV01 screenshots (toggle-boxes) are identical. I could only propose that the reason was in a font difference. But I compared fonts earlier: they are identical. Could it be the result of antialiasing or something similar? If not what could be the other reason?

#9 Updated by Vadim Gindin over 5 years ago

Here are superposing of P2J WEB over SWING:

and P2J WEB over WINDEV01:

Have a note on the MENUBAR first item: "SM2 Submenu".

In both screenshots I've superposed window title.
By the way menus are not fixed to fit by pixel-to-pixel to windev01, but at this task I'm working on difference between P2J SWING and WEB clients.

#10 Updated by Greg Shah over 5 years ago

By the way menus are not fixed to fit by pixel-to-pixel to windev01, but at this task I'm working on difference between P2J SWING and WEB clients.

Please make both clients work the same way as on windev01.

Could it be the result of antialiasing or something similar?

This is not likely. Anti-aliasing should not change the font metrics. It just smooths the rendering of the glyphs being displayed.

If not what could be the other reason?

Look carefully at the font metrics for the vertical dimensions which are used to draw the text. I suspect there is some difference there that we need to compensate for.

We have seen this problem also with other widget types, but it seems to be a bigger problem with toggle-box and menus. So there may be something we are doing in those widgets that makes it worse. This would be an additional issue to anything related to font metrics.

Constantin: do you have any other thoughts or guidance?

#11 Updated by Constantin Asofiei over 5 years ago

Vadim, disable anti-aliasing at the browser level (see here for details #2701-5) and after that see how the web client behaves.

Greg: anti-aliasing hurts us because for the web client we don't have a way to compute ascent/descent for a text; we compute full text height (by drawing text on a canvas and then looking for first and last row with a non-black pixel, and this we assume as "text-height") and if anti-aliasing is in effect, it will measure it, too. Thus, when centering a text vertically, we may end up in incorrect situations, as P2J assumes a certain text height, when in reality the browser assumes another text-height.

#12 Updated by Vadim Gindin over 5 years ago

After I've disabled anti-aliasing the fonts are now the same, but the problem is still remained.

#13 Updated by Vadim Gindin over 5 years ago

I've got actual text and font metrics and rechecked: the bug is still exists.

#14 Updated by Vadim Gindin about 5 years ago

Could you recall me, when I got actual text and font metrics will be the fonts equal? In my case they are similar but not equal...

#15 Updated by Constantin Asofiei about 5 years ago

Vadim, are the metrics of the toggle-box correct? I mean, the widget height/width (and its inner highlight rectangle), regardless of how the text is being drawn.

If they are OK, then (from the images you posted) there is a problem with determining the Y position of the text being drawn. For the Web client, we try to compute the text/font height in p2j.fonts.js:getTextHeight: the approach is to draw the text in a canvas and after that determine the lowest and highest point where the text is being drawn, and the result will be the text/font height. This may be different than how Swing computes it; from the javadoc of java.awt.FontMetrics.getHeight():

Gets the standard height of a line of text in this font. This is the distance between the baseline of adjacent lines of text. It is the sum of the leading + ascent + descent. Due to rounding this may not be the same as getAscent() + getDescent() + getLeading(). There is no guarantee that lines of text spaced at this distance are disjoint; such lines may overlap if some characters overshoot either the standard ascent or the standard descent metric.

Also, see some discussion here: http://stackoverflow.com/questions/1134586/how-can-you-find-the-height-of-text-on-an-html-canvas/12112978#12112978 .

Please debug into the drawing of toggle-box and menu widgets, and post here where the P2J Web and Swing positions the text (the X and Y coordinates).

#16 Updated by Greg Shah about 5 years ago

  • Target version changed from Milestone 12 to Milestone 16

#17 Updated by Vadim Gindin about 5 years ago

Menus font in P2J differs from Progress's font. I should probably hardcode menu font in P2J as it done for AlertBox or WindowTitle for example. Is some program that extracts system fonts for Windows already exists somewhere?

#18 Updated by Vadim Gindin about 5 years ago

Now I think that system fonts do not have fixed system ID (like colors for example). If I'm wrong - please correct me.

#19 Updated by Vadim Gindin about 5 years ago

I've faced with some difficulty during per-pixel comparing of SUBMENU bodies in Progress and P2J. Have a look at the screenshot:

SUB-MENU body is drawn in the following way: first we draw a background rectangle using gd.fill3DRect() and after that we call children drawing. Note that as a result of gd.fill3DRect() call we see that:
  1. left and top sizes (1 pixels thin) are white) in P2J, but in Progress white lines are shifted down and right. I.e. white lines are drawn at second pixels from the top and from the left.
  2. there are additional dark lines in Progress at the bottom and at the right.

I'm confused a little with that: is that a incorrect implementation of gd.fill3DRect() method or I should draw deficient lines myself right in SubMenuGuiImpl.draw()?

#20 Updated by Vadim Gindin about 5 years ago

Also note that SUB-MENU title (see SM2 Submenu for example in previous picture) rectangles in "pressed" state are also drawn differently in Progress and P2J. Colours of top/left and bottom/rigth sides are different correspondingly. Also corner pixels also have different colours. There are @gd.draw3DRectangle() calls. Is it a bug of 3D draw methods or not?

#21 Updated by Greg Shah about 5 years ago

I don't think this is a problem with 3D rectangle drawing. It is working exactly right in other use cases.

You just need to draw things properly yourself, to match the 4GL result. There are other issues too:

  • The color of the bottom and right lines are not right for the menu item "inset" drawing.
  • The bottom line of the menu item needs to be drawn 1 pixel more to the left.
  • There is a missing outer rectangle drawn in the 4GL for submenus.

#22 Updated by Vadim Gindin about 5 years ago

The branch is rebased to current trunk revision. Current branch revision is 11029.

#23 Updated by Vadim Gindin about 5 years ago

I've found not implemented menu behavior related to empty labels for CHUI.

  1. If label for menu-item or sub-menu is not set, than its name is used (same for GUI).
  2. If we set label to some value and after that set it again to ? then:
        ┌─────┐
        │ iii │
        │ _-> │
        └─────┘
    It is a SPACE is used to denote empty label (underscored as usual) for SUB-MENU and it works also as mnemonic.
  3. If sub-menu contains single menu-item with it's label manually set to ? (or several such menu-items) such sub-menu body even is not shown for CHUI. If sub-menu contains the child sub-menu with label manually set to ? then body must be shown.
  4. If in previous issue sub-menu contains more menu-items and some of them has empty label it is shown a single underlined SPACE, but mnemonic doesn't work.

Should I create a separate task for this or work here?

#24 Updated by Greg Shah almost 5 years ago

Should I create a separate task for this or work here?

These all seem easy to fix, right? Some of these seem like only a couple minutes to fix. Fix them here if they are easy. Create a new task if that is not correct.

#25 Updated by Vadim Gindin almost 5 years ago

I've faced with some problem after my refactoring:

The second item in menubar is overwritten somehow when I press on it (till I release the mouse button). It is a MENU-ITEM "print" and previous item is MENU-ITEM "qqq". Interesting but for "qqq" the bug is not happen.

Constantin, could you advice why it happen and how to debug it..?

#26 Updated by Vadim Gindin almost 5 years ago

I've committed current changes (sub-menus refactoring) and rebased the branch. Now current revision is 11037 (trunk rev 11026).

#27 Updated by Vadim Gindin almost 5 years ago

It seems this bug is not caused by menu classes.. Not sure, I've changed bgColor of them but the target rectangle didn't changed:

#28 Updated by Vadim Gindin almost 5 years ago

Here is the test to recreate the error:

def sub-menu sbmiiii
  menu-item miii label "iiii" /* accelerator "PgDn". */.
  menu-item qq label "qq".
  menu-item dfd label "dfd".
  menu-item sdsf label "sdsf".

def sub-menu sbmi
  menu-item miii label "iiii" /* accelerator "PgDn". */.

def sub-menu sbm
  menu-item mii label "iii" /* accelerator "PgDn". */.
  sub-menu sbmi label "S".

def menu m menubar
  menu-item qqq label "qqq".
  sub-menu sbm label "Orwell".
  sub-menu sbmiiii label "1984".
  menu-item print label "print".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

Or any other test with simple menubar.

Just press on menu-item "print" and you'll see that item is gone under some rectangle until you release the mouse and move the pointer out of the item.

#29 Updated by Constantin Asofiei almost 5 years ago

Vadim, please use this patch for AbstractGuiDriver.draw():

### Eclipse Workspace Patch 1.0
#P p2j
Index: src/com/goldencode/p2j/ui/client/gui/driver/AbstractGuiDriver.java
===================================================================
--- src/com/goldencode/p2j/ui/client/gui/driver/AbstractGuiDriver.java    (revision 819)
+++ src/com/goldencode/p2j/ui/client/gui/driver/AbstractGuiDriver.java    (working copy)
@@ -2069,8 +2069,8 @@
          // intersect the clipping rectangle with the screen bitmap's outer rectangle.  if the
          // intersection is null, do not draw anything
          NativeRectangle invalidation = ews.getScreenBitmap().getOuterRectangle();
-         NativeDimension clipdim = new NativeDimension(clip.width(), clip.height());
-         NativeRectangle rclip = new NativeRectangle(tempOrig, clipdim);
+         NativeRectangle rclip = new NativeRectangle(clip);
+         rclip = rclip.translate(tempOrig);

          if (invalidation != null)
          {

I think the assumption for most cases was that the clip argument is actually a dimension, not a rectangle (thus the origin of this rectangle was always (0,0)). So in your case, where the PaintEvent had the coordinates posted for the pressed menu-item, and the MENU widget was trying to limit drawing only for that menu-item (see MenuGuiImpl.draw - the if (!config().popupOnly) test) - after this, it was passing a clip rectangle with an origin different than (0,0).

Another issue is when you have a left-mouse pressed in a menu-item or in a opened menu, and you drag the mouse into another area outside of the initial click menu-item, it doesn't close the menu once the mouse was released.

#30 Updated by Vadim Gindin almost 5 years ago

Remaining issues:
  1. Closing menu, reported by Constantin
  2. Adjustment of sizes in SWING:
    1. sub-menu body height at this moment somehow depends on items count
    2. check item width depending on different options (checkmark, accelerator and so on)
    3. check popup menu
    4. check nested sub-menu sizes
  3. CHUI issues

#31 Updated by Vadim Gindin almost 5 years ago

I've finished adjustment of menu sizes and rebased the branch. Now current branch version 11075 (11048 trunk rev). Please review.

#32 Updated by Constantin Asofiei almost 5 years ago

Review for 2857a rev 11075:
  1. MenuItemConfig - is the implicit super-constructor call really needed? Isn't the default super-construct implicitly called? Explicitly calling super() makes the code easier to read, but I'm trying to understand what you were trying to fix.
  2. MenuItemGuiImpl, SubMenuGuiImpl - you have multiple history entries, merge them please.
  3. ToggleBoxGuiImpl - there are no functional changes, just a missed merged history entry

#33 Updated by Vadim Gindin almost 5 years ago

I've rebased the branch and fixed remarks.

Constantin Asofiei wrote:

Review for 2857a rev 11075:
  1. MenuItemConfig - is the implicit super-constructor call really needed? Isn't the default super-construct implicitly called? Explicitly calling super() makes the code easier to read, but I'm trying to understand what you were trying to fix.

Probably nothing, I'd just had some doubt that super constructor is called. reverted.

  1. MenuItemGuiImpl, SubMenuGuiImpl - you have multiple history entries, merge them please.

fixed.

  1. ToggleBoxGuiImpl - there are no functional changes, just a missed merged history entry

Probably I'd incorrectly merge this file. reverted.

Current branch revision is 11078.

#34 Updated by Vadim Gindin almost 5 years ago

Can I run regression testing?

#35 Updated by Greg Shah almost 5 years ago

Please rebase first. Then you can run regression testing.

#36 Updated by Vadim Gindin almost 5 years ago

Regression testing is finished. After single check the only one test remained failed: tc_item_master_007. Is it a known bug?

#37 Updated by Vadim Gindin almost 5 years ago

Here is the failing screen:

06/23/2016                  CYCLE COUNT UPDATE FOR GSO                  02:33:06                                    
  ┌─────────────────────────────────────────────────────────────────────────┐                                       
  │Item            Fac / Loc            Turnarnd S  Cut Off Qty    Count Qty│                                       
  │--------------- -------------------- -------- - ------------ ------------│                                       
  │UA001608826     H1   A-016           0001BVFX C        2.000        1.000│                                       
  │UA001610481     H1           ┌SITE SELECTION┐ C        0.000        1.000│                                       
  │UA001610481     H1           │Site: GSO     │ C        2.000        0.000│                                       
  │UA002800596     H1           └──────────────┘ C        2.000        0.000│                                       
  │UA004420006     H1                   0001H31F N        1.000            ?│                                       
  │UA004420006     H1                   0001HEG7 N        1.000            ?│                                       
  │UA004810462     H1                   0001H31J C        1.000        1.000│                                       
  │UA005600074     H1                            C        0.000        1.000│                                       
  │UA005631601     H1                            C        0.000        0.000│                                       
  │UA005631601     H1                   0001J39S C        1.000        1.000│                                       
  │UA005810023     H1                   0001DTRR C        1.000        1.000│                                       
  │UA005810023     H1                   0001HZNE C        2.000        1.000│                                       
  │UA007514142     H1                   0001c3e9 C        1.000        1.000│                                       
  │UA007514142     H1                   0001c8v3 C        2.000        1.000│                                       
  │UA007621075     H1                   0001HA25 C        8.000        1.000│                                       
  │UA007621075     H1                   0001HD9L C       12.000        1.000│                                       
  └─────────────────────────────────────────────────────────────────────────┘                                       
Enter Site to Work or ? for browser                                                                                 

   FAILED 180.106                                                                                                   
timeout before the specific screen buffer became available (Mismatched data at line 4, column 49. Expected 'N' (0x00
4E at relative Y 4, relative X 49) and found 'C' (0x0043 at relative Y 4, relative X 49), template: screens/tc/item/
master/007/item_master_007_step1.txt.)  

And here is the specified template:
12/08/2009                  CYCLE COUNT UPDATE FOR GSO                  19:05:46
  ┌─────────────────────────────────────────────────────────────────────────┐
  │Item            Fac / Loc            Turnarnd S  Cut Off Qty    Count Qty│
  │--------------- -------------------- -------- - ------------ ------------│
  │UA001608826     H1   A-016           0001BVFX N        2.000            ?│
  │UA001610481     H1           ┌SITE SELECTION┐ N        0.000            ?│
  │UA001610481     H1           │Site: GSO     │ N        2.000            ?│
  │UA002800596     H1           └──────────────┘ N        2.000            ?│
  │UA004420006     H1                   0001H31F N        1.000            ?│
  │UA004420006     H1                   0001HEG7 N        1.000            ?│
  │UA004810462     H1                   0001H31J N        1.000            ?│
  │UA005600074     H1                            N        0.000            ?│
  │UA005631601     H1                            N        0.000            ?│
  │UA005631601     H1                   0001J39S N        1.000            ?│
  │UA005810023     H1                   0001DTRR N        1.000            ?│
  │UA005810023     H1                   0001HZNE N        2.000            ?│
  │UA007514142     H1                   0001c3e9 N        1.000            ?│
  │UA007514142     H1                   0001c8v3 N        2.000            ?│
  │UA007621075     H1                   0001HA25 N        8.000            ?│
  │UA007621075     H1                   0001HD9L N       12.000            ?│
  └─────────────────────────────────────────────────────────────────────────┘

Enter Site to Work or ? for browser

As we can see the values in the column "S" is different.

#38 Updated by Constantin Asofiei almost 5 years ago

I'm not sure, but this one might be dependent on the DB state. Please do one more run.

#39 Updated by Vadim Gindin almost 5 years ago

The second run has finished and this test has passed and there are no other failed tests. Can I commit the changes?

#40 Updated by Vadim Gindin almost 5 years ago

I've rebased the branch just now with the latest trunk revisions (11052). Current revision is 11081.

#41 Updated by Greg Shah almost 5 years ago

Yes, go ahead and merge 2857a to trunk.

Are there any remaining issues left open in this task (e.g. the ChUI issues)?

#42 Updated by Vadim Gindin almost 5 years ago

The changes passed regression testing and committed to trunk as revision 11053.

#43 Updated by Vadim Gindin almost 5 years ago

The CHUI issues are left in this task.

#44 Updated by Greg Shah almost 5 years ago

Please create a separate task for those. Do not work them right now. I will close this task when the new task is opened.

#45 Updated by Vadim Gindin almost 5 years ago

I've created the task #3134 for CHUI issues.

#46 Updated by Greg Shah almost 5 years ago

  • % Done changed from 0 to 100
  • Status changed from New to Closed
  • Assignee set to Vadim Gindin

#47 Updated by Greg Shah over 4 years ago

  • Target version changed from Milestone 16 to Cleanup and Stabilization for GUI

Also available in: Atom PDF