Bug 194278 - Ability to customize FlowUtilities used by TextFlow and ParagraphTextLayout
Summary: Ability to customize FlowUtilities used by TextFlow and ParagraphTextLayout
Status: RESOLVED FIXED
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: 3.2.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4.0 (Ganymede) M2   Edit
Assignee: Cherie Revells CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-06-25 14:51 EDT by Cherie Revells CLA
Modified: 2008-09-18 13:31 EDT (History)
3 users (show)

See Also:


Attachments
suggested fix (36.19 KB, patch)
2007-08-02 16:48 EDT, Cherie Revells CLA
no flags Details | Diff
updated patch with no whitespace (11.76 KB, patch)
2007-09-05 11:16 EDT, Cherie Revells CLA
no flags Details | Diff
final patch (40.52 KB, patch)
2007-09-19 11:18 EDT, Cherie Revells CLA
ahunter.eclipse: iplog+
Details | Diff
Updated javadoc (4.15 KB, patch)
2007-09-24 09:29 EDT, Cherie Revells CLA
ahunter.eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Cherie Revells CLA 2007-06-25 14:51:21 EDT
In GMF, we allow clients to use measurement units other than pixels.  Some of the classes in org.eclipse.draw2d.text require customization as size calculation uses font size in pixels.  The classes 
ParagraphTextLayout and TextFlow use some methods from FlowUtilities which use pixels.  These methods specifically are the problems in FlowUtilities:
getAverageCharWidth, measureString, and setupFragment.  I would like to refactor FlowUtilities so it can be customized (add non-static methods that can be overridden) and then pass an instance of this into the TextFlow and ParagraphTextLayout.  I will need to do this in a way that avoids breaking existing API.
Comment 1 Cherie Revells CLA 2007-08-02 16:48:40 EDT
Created attachment 75271 [details]
suggested fix
Comment 2 Cherie Revells CLA 2007-08-02 16:49:38 EDT
See my attached patch for my suggested implementation.  I also changed the visibility of a few things that I require access to in GMF and that seem like they would be stable to me.
Comment 3 Anthony Hunter CLA 2007-08-13 16:56:27 EDT
We are waiting for Randy to review the patch.
Comment 4 Cherie Revells CLA 2007-09-05 11:16:19 EDT
Created attachment 77732 [details]
updated patch with no whitespace

This one is ready to be committed, I just need a code review.
Comment 5 Randy Hudson CLA 2007-09-05 22:23:03 EDT
This patch would expose some ugly and unorganized internals which could make it hard to improve the text package in the future. For example, to determine if wrapping is necessary in a block whose width is not constant (such as HTML page with floating DIVs), the LookAhead should be providing ascent and descent information, but it doesn't. I don't really understand the use case here. When is a Font's ascent not what SWT says it is?  Also, isn't SWT on the brink of supporting floating point font heights?
Comment 6 Cherie Revells CLA 2007-09-06 10:25:42 EDT
In GMF, we allow the client to use any logical measurement unit when drawing on the diagram.  This way clients could size their figures so that they are not dependent on the resolution of the screen (by having one logical unit equal 0.001 millimetre for example).

We use a ScaledGraphics object that translates the logical units to pixels when painting occurs.  The ScaledGraphics object expects all paint/draw methods to pass in the logical measurement unit.  So, if a client was using himetric mapmode, for example, they would always use himetric units when painting their figures.

Any GEF class that assumes pixels are the measurement unit being used need to be overridden in GMF.  We have already done this is some cases (e.g. Label, ImageFigure).  For the text flow figures, we need to override any code that does calculations assuming the measurement unit is in pixels (e.g. any calls that use the values from the FontMetrics class are in pixels).

Overriding the necessary code in the flow figure classes in GMF would involve too much code copying from GEF.  Therefore, we need to expose some of this API so that it can be overridden more cleanly in GMF.  
Comment 7 Cherie Revells CLA 2007-09-19 11:18:35 EDT
Created attachment 78759 [details]
final patch

Updated patch reviewed by Anthony.
Comment 8 Randy Hudson CLA 2007-09-19 11:51:34 EDT
> Any GEF class that assumes pixels are the measurement unit being used need to
> be overridden in GMF.  We have already done this is some cases (e.g. Label,
> ImageFigure).  For the text flow figures, we need to override any code that
> does calculations assuming the measurement unit is in pixels (e.g. any calls
> that use the values from the FontMetrics class are in pixels).

This type of mapping is done more easily in the layout (using scaled constraints), editpart (pre-scaling the constraints or font heights), and/or zoom manager (using the UI multiplier). SWT only supports pixel-based painting so I don't see how any other unit could be supported by SWT/draw2d.  I realize the Fonts are created using "point" instead of "pixel", but mapping to the desired *pixel* size can be done in the editpart rather than in the figure (since it is closer to the SWT layer).
Comment 9 Anthony Hunter CLA 2007-09-19 14:11:45 EDT
Committed to HEAD
Comment 10 Remy Suen CLA 2007-09-22 22:15:31 EDT
How come org.eclipse.draw2d.Label's getTextUtilities() method is tagged with '@since 3.2' instead of '@since 3.4'? TextFlow's getFlowUtilities() and getTextUtilities() seems to have this problem too. There also seems to be some other methods that have similar issues (private -> protected with no API documentation).
Comment 11 Cherie Revells CLA 2007-09-24 09:29:36 EDT
Created attachment 79072 [details]
Updated javadoc

I fixed the javadoc and realized a couple methods in FlowUtilities could be made final.