version 2.20.1 by jfboeve · Pull Request #604 · rdkcentral/Lightning

jfboeve · 2026-02-05T11:21:19Z

No description provided.

RTL layout support

Fix text texture RTL propagation

Fix advanced renderer text alignment mirroring

Fix ES5 package

- allow disabling truncation (Arabic) - WIP wordBreak (normal renderer) - WIP unit tests

Bidirectional text rendering (RTL)

Fix canvas 2d renderer mirroring

Feature: expose lines layout in renderInfo

Font accuracy fix

This reverts commit 2319751.

RTL CI troubleshooting

Feature/rtl

rdkcmf-jenkins · 2026-02-05T11:22:28Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

Protex Server Path: /home/blackduck/github/Lightning/604/rdkcentral/Lightning
Commit: 278b0e7

Report detail: gist'

src/textures/TextTokenizer.ts

Copilot

Pull request overview

This PR introduces comprehensive Right-to-Left (RTL) layout support and significantly refactors the text rendering system in Lightning Core. The changes enable proper rendering and layout of RTL languages like Arabic and Hebrew, including bidirectional text support.

Changes:

Added RTL layout support with automatic mirroring of element positioning and input handling
Complete refactoring of text rendering system from .mjs to TypeScript with new modular architecture
New bidirectional text tokenization system with optional bidi-js integration
Performance improvements including optimized idle loop handling and alpha-based rendering optimization

Reviewed changes

Copilot reviewed 51 out of 53 changed files in this pull request and generated 22 comments.

Show a summary per file

File	Description
package.json	Version update to 2.17.0-beta.6, added bidi-js dependency and bidiTokenizer export
vite.config.js	Added BUILD_BIDI_TOKENIZER build configuration and test exclusions
src/tree/core/ElementCore.mjs	Implemented RTL layout calculations and direction propagation
src/tree/Element.mjs	Added RTL property getter/setter and direction change hooks
src/application/Application.mjs	Added direction-aware event routing for Left/Right key swapping
src/textures/TextTexture*.ts	Complete rewrite of text rendering in TypeScript with modular architecture
src/textures/bidiTokenizer.ts	New bidirectional text tokenizer for RTL language support
src/textures/TextTokenizer.ts	New pluggable text tokenization system
src/renderer/c2d/shaders/DefaultShader.mjs	Added texture mirroring support for Canvas2D renderer
src/platforms/browser/WebPlatform.mjs	Optimized idle loop handling and removed redundant code
tests/*	Added comprehensive tests for RTL layout, text mirroring, and bidirectional rendering
docs/*	Added RTL documentation and updated text rendering guides
playwright.config.ts	Added font rendering flags for consistent screenshot testing

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-05T11:26:07Z

tests/textures/test.text.js

+                    app.children = [element];
+                    const texture = app.tag("Item").texture;
+                    stage.drawFrame();
+                    console.log(texture.source.renderInfo.lines);


Console.log statements should be removed before merging to production. These appear to be debug statements left in the test code.

Copilot · 2026-02-05T11:26:07Z

src/application/Application.mjs

        for (let i = n - 1; i >= 0; i--) {
-            const event = path[i]._getMostSpecificHandledMember(events);
+            const target = path[i];
+            const events = isHorizontalDirection && target.rtl ? eventsRtl : eventsLtr;


Variable shadowing: the events parameter is shadowed by the local events variable on line 336. This makes the code confusing and could lead to bugs. Consider renaming the local variable to something like targetEvents or resolvedEvents.

Copilot · 2026-02-05T11:26:07Z

tests/textures/test.text.js

+                    app.children = [element];
+                    const texture = app.tag("Item").texture;
+                    stage.drawFrame();
+                    console.log(texture.source.renderInfo.lines);


Console.log statements should be removed before merging to production. These appear to be debug statements left in the test code.

Copilot · 2026-02-05T11:26:07Z

src/renderer/c2d/shaders/DefaultShader.mjs

+                let mirroring = sourceW < 0 || sourceH < 0;
+                if (mirroring) {
+                    ctx.save();
+                    if (sourceW < 0) {
+                        drawSourceX = sourceX + sourceW;
+                        drawSourceW = -sourceW;
+                        ctx.scale(-1, 1);
+                        destX = -vc.w;
+                    }
+                    if (sourceH < 0) {
+                        drawSourceY = sourceY + sourceH;
+                        drawSourceH = -sourceH;
+                        ctx.scale(1, -1);
+                        destY = -vc.h;
+                    }
+                }


The mirroring logic uses sourceW < 0 || sourceH < 0 to detect mirroring, but doesn't handle the case where the texture dimensions might legitimately be negative for other reasons (if that's possible). Additionally, when both width and height are negative, the scale operations ctx.scale(-1, 1) and ctx.scale(1, -1) are applied separately. These should be combined into a single ctx.scale(-1, -1) for clarity and potential performance improvement.

No textures aren't meant to have negative dimensions

Copilot · 2026-02-05T11:26:07Z

tests/textures/test.text.js

                    stage.drawFrame();
                    chai.assert(texture.source.renderInfo.lines.length === 10);
-                    chai.assert(texture.source.renderInfo.lines.slice(-1)[0].substr(-6) == 'eget..');
+                    chai.assert(getLineText(texture.source.renderInfo.lines.slice(-1)[0]).substr(-5) == 'neq..');


The expected suffix changed from 'eget..' (6 characters) to 'neq..' (5 characters), but the substr(-6) was changed to substr(-5). This suggests the expected text changed. Please verify this is the correct expected value for this test case, as this appears to be a behavioral change rather than just a refactoring.

Copilot · 2026-02-05T11:26:10Z

src/textures/TextTextureRendererAdvancedUtils.ts

+          let { text, width, style, rtl } = last;
+          if (style) {
+            ctx.font = style.font;
+          }
+          const reversed = primaryRtl !== rtl;
+          do {
+            text = reversed ? trimWordStart(text, rtl) : trimWordEnd(text, rtl);
+            width = measureText(ctx, text, letterSpacing);
+          } while (width > maxWidth);


The initial value of width is unused, since it is always overwritten.

Suggested change

let { text, width, style, rtl } = last;

if (style) {

ctx.font = style.font;

}

const reversed = primaryRtl !== rtl;

do {

text = reversed ? trimWordStart(text, rtl) : trimWordEnd(text, rtl);

width = measureText(ctx, text, letterSpacing);

} while (width > maxWidth);

let { text, style, rtl } = last;

if (style) {

ctx.font = style.font;

}

const reversed = primaryRtl !== rtl;

let width = measureText(ctx, text, letterSpacing);

while (width > maxWidth) {

text = reversed ? trimWordStart(text, rtl) : trimWordEnd(text, rtl);

width = measureText(ctx, text, letterSpacing);

}

No we want to use the value in last instead of calling measureText for the initial value

Copilot · 2026-02-05T11:26:11Z

vite.config.js

+  entry = resolve(__dirname, 'src/textures/bidiTokenizer.ts');
+  outputBase = 'bidiTokenizer';
+  sourcemap = true;
+  useDts = true;


The value assigned to useDts here is unused.

Copilot · 2026-02-05T11:26:11Z

src/textures/bidiTokenizer.ts

+
+// https://www.unicode.org/reports/tr9/
+const reZeroWidthSpace = /[\u200B\u200E\u200F\u061C]/;
+const reDirectionalFormat = /[\u202A\u202B\u202C\u202D\u202E\u202E\u2066\u2067\u2068\u2069]/;


Character '\u202E' is repeated in the same character class.

Copilot · 2026-02-05T11:26:11Z

src/textures/bidiTokenizer.ts

+          break;
+        }
+      }
+    }


Avoid automated semicolon insertion (97% of all statements in the enclosing function have an explicit semicolon).

Copilot · 2026-02-05T11:26:11Z

src/textures/TextTextureRendererUtils.ts

+      };
+    default:
+      return {
+        suffix: textOverflow || maxLinesSuffix,


This use of variable 'textOverflow' always evaluates to true.

Suggested change

suffix: textOverflow || maxLinesSuffix,

suffix: textOverflow,

michielvandergeest

LGTM

michielvandergeest · 2026-02-05T11:33:53Z

I see we have a blackduck violation, assuming it's not an actual violation I'll see into getting it waivered

Address some Copilot feedback

Copilot

Pull request overview

Copilot reviewed 51 out of 53 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-05T15:27:09Z

src/application/Application.mjs

+            const events = isHorizontalDirection && target.rtl ? eventsRtl : eventsLtr;
+            const event = target._getMostSpecificHandledMember(events);


Variable name collision: On line 336, the variable events shadows the parameter events from line 326. This could lead to confusion. Consider renaming the local variable to targetEvents to match the pattern used in focusTopDownEvent on line 310.

Suggested change

const events = isHorizontalDirection && target.rtl ? eventsRtl : eventsLtr;

const event = target._getMostSpecificHandledMember(events);

const targetEvents = isHorizontalDirection && target.rtl ? eventsRtl : eventsLtr;

const event = target._getMostSpecificHandledMember(targetEvents);

Copilot · 2026-02-05T15:27:10Z

src/textures/TextTextureRendererUtils.ts

+      if (wordBreak && wordWidth > wrapWidth) {
+        const broken = breakWord(context, word, wrapWidth, letterSpacing);
+        let last = broken.pop()!;
+        for (const k of broken) {
+          resultLines.push({
+            text: k.text,
+            width: k.width,
+          });
+        }
+        result = last.text;
+        wordWidth = last.width;
+      }


The word breaking logic on line 126-137 can produce more lines than maxLines, as noted in the comment. After breaking words, the code should check if resultLines.length >= maxLines and break early if needed, to avoid exceeding the specified limit.

Copilot · 2026-02-05T15:27:10Z

src/textures/TextTextureRendererAdvancedUtils.ts

+        const maxWidth = maxLineWidth - line.width;
+
+        if (allowTruncation && maxWidth > 0) {
+          let { text, width, style, rtl } = last;


The initial value of width is unused, since it is always overwritten.

Suggested change

let { text, width, style, rtl } = last;

let { text, style, rtl } = last;

let width: number;

elsassph and others added 30 commits April 10, 2025 09:46

RTL layout support

6f9b72b

set beta version

39060ae

Merge pull request #568 from elsassph/feat/rtl-layout

92baf58

RTL layout support

Fix text texture RTL propagation

5bf481b

Merge pull request #570 from elsassph/fix/rtl-text-propagation

a8d1d4f

Fix text texture RTL propagation

Fix ES5 package

f8d949b

Fix advanced renderer text alignment mirroring

fe1944b

Merge pull request #572 from elsassph/fix/advanced-rtl-align

9c5cffd

Fix advanced renderer text alignment mirroring

Bumped version to 2.16.0-beta.1.

a711b56

Merge pull request #571 from elsassph/fix/es5-compat

9a9d7e3

Fix ES5 package

New text texture renderers with opt-in bidi

a66ae41

Improvements

19caa4a

- allow disabling truncation (Arabic) - WIP wordBreak (normal renderer) - WIP unit tests

wordBreak

db8b200

letter spacing advanced

7588e9d

linter

1487e66

Keep under max texture width

af9585a

Merge pull request #574 from elsassph/feature/bidi-text

a72101a

Bidirectional text rendering (RTL)

support negative clipping coordinates in c2d renderer

9e3b73f

integration tests

3b99c74

ensure calculations are correct when scaled

aa718a5

test cleanup

2256656

vertical mirroring

cc73977

test cleanup

079381b

cleanup redundancy

364061e

tweak automation

cc99f56

Merge pull request #576 from elsassph/fix/c2d-mirroring

5788277

Fix canvas 2d renderer mirroring

Modify renderInfo to expose more layout data

0e72973

version bump

9565014

Merge pull request #577 from elsassph/feature/rtl-renderinfo

8c446f3

Feature: expose lines layout in renderInfo

Fix ASCII detection

72f481c

jfboeve and others added 9 commits February 3, 2026 10:07

Merge pull request #601 from elsassph/fix/ci-fonts-letterspacing

63e08c5

Font accuracy fix

remove launch opts

2319751

restore opts

0f1bcd0

try looks-same options

a87572c

Revert "remove launch opts"

43359a1

This reverts commit 2319751.

set headless display

fb8fd4f

try looks-same options

949eb11

Merge pull request #602 from elsassph/feature/rtl-ci

6788ad3

RTL CI troubleshooting

Merge pull request #573 from rdkcentral/feature/rtl

278b0e7

Feature/rtl

Copilot AI review requested due to automatic review settings February 5, 2026 11:21

Copilot started reviewing on behalf of jfboeve February 5, 2026 11:21 View session

github-advanced-security bot found potential problems Feb 5, 2026

View reviewed changes

src/textures/TextTokenizer.ts Fixed Show fixed Hide fixed

Copilot AI reviewed Feb 5, 2026

View reviewed changes

michielvandergeest previously approved these changes Feb 5, 2026

View reviewed changes

elsassph and others added 2 commits February 5, 2026 15:22

Address some Copilot feedback

d59b933

Merge pull request #605 from elsassph/fix/copilot-feedback

06c7e0e

Address some Copilot feedback

michielvandergeest dismissed their stale review via 06c7e0e February 5, 2026 14:42

jfboeve and others added 2 commits February 5, 2026 16:17

Merge branch 'master' into dev

2701eb0

2.20.0

366f92f

Copilot AI review requested due to automatic review settings February 5, 2026 15:23

Copilot started reviewing on behalf of jfboeve February 5, 2026 15:23 View session

2.20.1

2f579a2

jfboeve changed the title ~~version 2.20.0~~ version 2.20.1 Feb 5, 2026

Copilot AI reviewed Feb 5, 2026

View reviewed changes

michielvandergeest self-requested a review February 5, 2026 15:32

michielvandergeest approved these changes Feb 5, 2026

View reviewed changes

jfboeve merged commit 461fe00 into master Feb 5, 2026
12 checks passed

github-actions bot locked and limited conversation to collaborators Feb 5, 2026

	suffix: textOverflow \|\| maxLinesSuffix,
	suffix: textOverflow,

		const events = isHorizontalDirection && target.rtl ? eventsRtl : eventsLtr;
		const event = target._getMostSpecificHandledMember(events);

	let { text, width, style, rtl } = last;
	let { text, style, rtl } = last;
	let width: number;

Conversation

jfboeve commented Feb 5, 2026

Uh oh!

rdkcmf-jenkins commented Feb 5, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

elsassph Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

elsassph Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

michielvandergeest left a comment

Choose a reason for hiding this comment

Uh oh!

michielvandergeest commented Feb 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants