feat(text): Shape complex single line UI text - #3231
Conversation
PR Summary by QodoShape complex single-line UI text with Uniscribe
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
| Filename | Overview |
|---|---|
| Core/Libraries/Source/WWVegas/WW3D2/render2dsentence.cpp | Implements complex-text eligibility, Uniscribe layout and rasterization, bounded surface allocation, texture chunking, and legacy fallback; the previously reported oversized-text failures are no longer reachable. |
| Core/Libraries/Source/WWVegas/WW3D2/render2dsentence.h | Extends the sentence renderer and font interfaces with complex-text measurement, rasterization, enablement, and fallback-result reporting. |
| Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp | Resolves sentence construction before returning shaped dimensions and replaces cached extents when rendering falls back to the legacy path. |
| GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplayString.cpp | Mirrors the Generals display-string fallback and layout-ordering fixes. |
| Core/GameEngine/Source/GameClient/GUI/GameWindowManager.cpp | Explicitly keeps editable and IME-related display strings on the legacy renderer until shaped caret metrics are supported. |
| Core/Libraries/Source/WWVegas/WW3D2/ww3d.cpp | Unloads the runtime Uniscribe entry points during WW3D shutdown. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Display string text changes] --> B{Eligible complex single line?}
B -- No --> C[Legacy per-character measurement and rendering]
B -- Yes --> D[Measure shaped paragraph]
D --> E{Dimensions supported?}
E -- No --> C
E -- Yes --> F[Rasterize shaped visual runs]
F --> G{Rasterization succeeded?}
G -- No --> C
G -- Yes --> H[Split raster across sentence textures]
H --> I[Draw shaped sentence]
C --> J[Publish legacy extents]
I --> K[Publish shaped extents]
Reviews (14): Last reviewed commit: "feat(text): Shape complex single-line UI..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a852d41fbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
I can't tell from the text and images what the problems were and how this fixes it.
Can you give before and afters of each of these individually? |
will provide examples of it as soon as possible |
the Arabic text as for now! before
after
|
f88c715 to
d25f054
Compare
d25f054 to
eff4156
Compare
|
draft to fix the vc6 issue |
|
We can handle VC6 in a small prerequisite PR by runtime-loading |
eff4156 to
74c1e27
Compare
|
Code review by qodo was updated up to the latest commit 74c1e27 |
yes im working on it! |
|
In what shape is the arabic text you tested with? As far as I am aware the old translations had the words reversed to accomodate the game implementation. Can arabic text now be supplied normally? |
i used noraml text arabic, i didnt reverse the text, this fixes the hack that is to reverse the arabic text! |
|
needs rebase and conflict fix, working on it. |
a845dfe to
80cebad
Compare
|
Rebased and fixed and addressed all the changes required |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80cebad841
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
80cebad to
60951d4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60951d4c23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a5a59b9 to
42b2337
Compare
42b2337 to
2220c79
Compare
4985fdf to
c986ded
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Rebased! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c986dedf67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| hotkey.translate(TheHotKeyManager->searchHotKey(getText())); | ||
| } | ||
|
|
||
| const Bool hasHotkey = !hotkey.isEmpty(); |
There was a problem hiding this comment.
When a hotkey-enabled complex label contains a literal ampersand followed by whitespace (for example, Arabic text containing &), this still sets hasHotkey: the current HotKeyManager::searchHotKey() returns the character after any & without validating it (Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp:211-218), while the sentence renderer consumes the marker only when the following character is greater than L' ' (render2dsentence.cpp:1150). ParseHotKey therefore remains true and Is_Single_Line_Complex_Text() rejects the whole label, leaving its Arabic text unshaped. This predicate mismatch is fresh evidence beyond the resolved no-marker case; derive hasHotkey using the renderer's accelerator-validity rules.
Useful? React with 👍 / 👎.
c986ded to
f881a12
Compare
5 participants
Adds contextual shaping and bidirectional ordering for complex single-line UI text in
Render2DSentenceClass.The existing sentence renderer processes text one
WCHARat a time, which prevents Arabic letters from using their contextual forms and breaks the visual order of mixed Arabic and Latin text. Eligible strings are now itemized as one paragraph, divided into visually ordered font runs only at character-cluster boundaries, and rasterized once before being copied across the existing A4R4G4B4 sentence textures.Plain Latin strings continue to use the existing per-character renderer. Multiline text, text requiring wrapping, text with an active hot-key marker, monospaced text, and editable text entries remain on the legacy path.
The required Uniscribe entry points are loaded at runtime through #3241. If Uniscribe is unavailable, rendering falls back to the legacy path.
Before
After
The change was validated with:
git diff --checkThe implementation was developed with AI assistance, then manually reviewed and simplified against the nearby renderer and runtime-loader code.