Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text annotation APIs #6572

Merged
merged 148 commits into from
Apr 17, 2024
Merged

Text annotation APIs #6572

merged 148 commits into from
Apr 17, 2024

Conversation

pmconne
Copy link
Member

@pmconne pmconne commented Mar 29, 2024

imodel-native: iTwin/imodel-native#718

This PR introduces support for creating and manipulating blocks of text, laying them out, and converting them to geometric primitives. It also provides straightforward APIs for querying and updating the text blocks stored on TextAnnotation2d/3d elements and updating their geometry streams.

It introduces the following APIs:

TextStyle (core-common)

Represents a text style obtained from a workspace. The TextStyle elements created by batch connectors are ignored. Text styles are identified by name.

TextBlock (core-common)

In-memory representation of a block of text, with content and styling. The wire format and persistence format are the same - JSON.

TextBlocks refer to fonts by name, not Id.

produceTextAnnotationGeometry (core-backend)

Processes a TextAnnotation and splits it into lines with word-wrapping. It then outputs primitive commands, namely: color changes, TextStrings, and fraction separators. This can be used to append a TextBlock to a geometry stream e.g. via GeometryStreamBuilder.appendTextBlock.

This must be in core-backend because 1) it needs access to fonts and text styles stored in workspaces and font Ids stored in the iModel's font table; 2) it needs the native freetype library to layout glyphs; and 3) it needs the Intl.Segmenter API that despite being a Stage 4 ECMAScript proposal implemented in most other browsers and Node since 2020, is still not generally available in Firefox.

TextAnnotation (core-common)

A TextBlock positioned in 2d or 3d space, typically attached to an element, though you can also use it to produce decoration graphics.

The BisCore schema and native code define an optional frame and leaders as part of the annotation. No one has ever used them, so they're omitted here, but they could be introduced in the future if needed without breaking the API. I expect dimensions will have their own representations of these, though.

TextAnnotation elements (core-backend)

These elements used to store their annotation in a separate aspect as a binary flatbuffer-encoded blob. Now, they store it directly as JSON in their jsonProperties field. A setAnnotation method updates this property and recomputes the geometry stream accordingly.

dta text keyin

A lazy, quick-and-dirty, append-only text editing keyin. You can invoke the keyin with different commands+arguments in succession to build up a TextBlock displayed as a decoration graphic in the current viewport.


TODO

  • Fix numerator drawing underneath horizontal fraction separator
  • Fix numerator and denominator not aligned on center of horizontal separator
  • Add a very simple key-in to place a block of text
  • Test against real fonts

Follow-up

  • Implement lookup of text styles from workspaces.
  • Deprecate BisCore classes like TextAnnotationData.
  • Delete native annotation APIs and the callback that recomputes an annotation element's geometry stream when the annotation aspect is modified.

@kabentley
Copy link
Contributor

re font names:

  • for styles there's no Id, since that is a table in an iModel. Therefore, i think we should migrate to using font name only in new apis.
  • font names aren't quite unique if we support different font types. My suggestion is to ignore this and resolve by name using TT, then SHX, then RSC (if we even support SHX/RSC). In other words, if you have two fonts with the same name, the TT one will be used.
  • font names should be case insensitive (right @jffmarker )
  • some fonts can be found by more than one name. I don't see why that's a problem as long as you always find the same font for a given name
  • lots more font name/family quirks when we attempt to resolve a "missing" font, but i don't think they're worth worrying about here.

@kabentley
Copy link
Contributor

even though existing connectors create Text Style Elements and TextAnnotationData and even refer to them by Id in other elements, nothing anywhere, including the connectors themselves can be using them for anything, right? I don't think we need to maintain any compatibility with them and can declare their types "obsolete".

@pmconne
Copy link
Member Author

pmconne commented Mar 29, 2024

font names should be case insensitive

They are not treated as such by FontMap.

@kabentley
Copy link
Contributor

kabentley commented Mar 29, 2024

They are not treated as such by FontMap.

Yup, that's a bug. They're required to be unique case insensitively.

Surely if you search for "arial" you should find "Arial".

@pmconne
Copy link
Member Author

pmconne commented Mar 29, 2024

unique case insensitively.

Eww, using SQLite's definition of "case-insensitive"?

the 26 upper case characters of ASCII are folded to their lower case equivalents before the comparison is performed. Note that only ASCII characters are case folded. SQLite does not attempt to do full UTF case folding

@pmconne
Copy link
Member Author

pmconne commented Mar 30, 2024

@aruniverse @ben-polinsky our release tags guidelines make conflicting statements about the @preview tag:

A lint rule enforces that the "extensions" tag may only be applied to APIs tagged as "public" or "preview".

"The "preview" tag is supposed to be the equivalent of "beta" for extension APIs. It is currently unused, and may never be.

Preview API tag should be on its own line before the @extensions tag:

@preview is used nowhere in core-common nor core-frontend. My APIs are tagged @beta and @extensions, and they produce the following error:

/home/paul/c/js/s/itwinjs/core/common/src/annotation/TextBlock.ts
   15:1  error  interface "ApplyTextStyleOptions" without one of the release tags "public,preview"      @itwin/public-extension-exports

However, if I add @preview, the error is resolved. So the comment about "preview" being currently unused seems inaccurate.

I still don't understand why we need both "beta" and "preview".

@kabentley
Copy link
Contributor

kabentley commented Mar 30, 2024

Eww, using SQLite's definition of "case-insensitive"?

We are supposed to use the ICU extension for that. We ship it, but I don't see where it gets hooked up. Looks to me like that got lost somewhere. @khanaffan @chuckkir @jffmarker

This isn't that important for font names, but is definitely required for code values.

@chuckkir
Copy link

chuckkir commented Apr 1, 2024

We are supposed to [use the ICU extension] for that. We ship it, but I don't see where it gets hooked up. Looks to me like that got lost somewhere. @khanaffan @chuckkir @jffmarker

It gets linked into itwinbentley.dll in bentley.mke:

 LINKER_LIBRARIES            = $(BuildContext)SubParts/Libs/$(libprefix)iTwinIcu4c$(libext)

It is also used by the itwinlibxml2 partfile, although I'm not sure if it is used in the DLL itself.

@pmconne pmconne requested a review from a team as a code owner April 16, 2024 17:36
@pmconne pmconne requested review from calebmshafer, wgoehrig and a team as code owners April 17, 2024 16:38
@pmconne pmconne enabled auto-merge (squash) April 17, 2024 16:42
@ben-polinsky
Copy link
Contributor

ben-polinsky commented Apr 17, 2024

@pmconne Looking into the documentation link issues which don't seem like typos:

GeometryStreamBuilder.appendTextAnnotation method does not exist?
Invalid Link: "[GeometryStreamBuilder.appendTextAnnotation]($common)" in file "reference/core-backend/elementgeometry/producetextannotationgeometry"

This may be a bug...
Should be ($common)
Invalid Link: "[Placement]($geometry)" in file "reference/core-backend/elements/textannotation2d"
Invalid Link: "[Placement]($geometry)" in file "reference/core-backend/elements/textannotation3d"

Word/camel hump order (ApplyTextStyleOptions):

TypeDoc Reference Link Error: Reference to 'TextStyleApplyOptions' was not found at File: core-common/core/common/src/annotation/TextBlock.ts Line: 81

Spelling (Workspace):

Invalid Link: "[Worksapce]($backend)" in file "reference/core-common/annotation/fractionrunprops"
Invalid Link: "[Worksapce]($backend)" in file "reference/core-common/annotation/linebreakrunprops"
Invalid Link: "[Worksapce]($backend)" in file "reference/core-common/annotation/paragraphprops"
Invalid Link: "[Worksapce]($backend)" in file "reference/core-common/annotation/textblockcomponentprops"
Invalid Link: "[Worksapce]($backend)" in file "reference/core-common/annotation/textblockprops"
Invalid Link: "[Worksapce]($backend)" in file "reference/core-common/annotation/textrunprops"

($backend)?:

Invalid Link: "[TextAnnotation2d]($common)" in file "reference/core-common/entities/textannotation2dprops"
Invalid Link: "[TextAnnotation3d]($common)" in file "reference/core-common/entities/textannotation3dprops"
Invalid Link: "[TextAnnotation2d]($common)" in file "reference/core-common/entities/"
Invalid Link: "[TextAnnotation3d]($common)" in file "reference/core-common/entities/"
Invalid Link: "[TextAnnotation2d]($common)" in file "reference/core-common/all/"
Invalid Link: "[TextAnnotation3d]($common)" in file "reference/core-common/all/"

@pmconne pmconne merged commit fd84cc9 into master Apr 17, 2024
14 checks passed
@pmconne pmconne deleted the pmc/textblock branch April 17, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet