Add `parlance::BidiLevel` by tomcur · Pull Request #710 · linebender/parley

tomcur · 2026-07-24T14:52:32Z

The motivation is to have a repr(transparent) wrapper for bidi levels to hold documentation and useful methods (e.g., letting users call level.is_rtl() instead of manually writing !level.is_multiple_of(2), or having our APIs vary between level() -> u8 and is_rtl() -> bool methods), while allowing bytemucking between BidiLevel <=> u8 for interop.

parley_engine now returns BidiLevel, but I've kept parley's API unchanged.

tomcur · 2026-07-24T14:56:10Z

    /// The base bidi level of the paragraph of text.
    #[inline(always)]
-    pub fn paragraph_level(&self) -> u8 {
+    pub fn paragraph_level(&self) -> BidiLevel {
        self.paragraph_level
    }
-
-    /// Whether the paragraph's resolved base direction is right-to-left.
-    #[inline(always)]
-    pub fn is_rtl(&self) -> bool {
-        !self.paragraph_level.is_multiple_of(2)
-    }


Removing Analysis::is_rtl in favor of Analysis::paragraph_level().is_rtl(), as per #708 (comment).

In that same thread, Bruce pointed out we have some inconsistency in the code base, sometimes calling this "base level", sometimes "paragraph level". The spec uses both words, but uses "paragraph level" a bit more. The code uses "base level" more. In any case, we probably should still rename this, either to something like base_bidi_level or paragraph_bidi_level.

The motivation is to have a `repr(transparent)` wrapper for bidi levels to hold documentation and useful methods (e.g., letting users call `level.is_rtl()` instead of manually writing `!level.is_multiple_of(2)`), while being able to bytemuck between `BidiLevel` <=> `u8` for interop.

tomcur · 2026-07-24T14:57:44Z

We could similarly implement bytemuck traits for NormalizedCoord (see #679 (comment) and #679 (comment)).

nicoburns

Very nice. Good use of parlance.

nicoburns · 2026-07-24T15:33:19Z

+#[inline(always)]
+const fn next_odd(level: BidiLevel) -> BidiLevel {
+    BidiLevel::new((level.to_u8() + 1) | 1)
+}
+
+#[inline(always)]
+const fn next_even(level: BidiLevel) -> BidiLevel {
+    BidiLevel::new((level.to_u8() + 2) & !1)
+}


These could optionally be methods on the type

The lack of capping to 125 here would be my main hesitance, but I think on balance I agree (also with it being optional/up to your discretion)

I've added the methods, with some documentation about wrapping.

DJMcNab

Nothing blocking, some code style nits.

I'm really happy to see this type properly documented. I did try and start on that task, but it got too big for what I had time to do.

This probably should have changelog entries (what is parley_engine's changelog story?)

DJMcNab · 2026-07-25T00:47:06Z

+/// These are numbers indicating how deeply bidirectional embeddings are nested in the text, and the
+/// default direction of text on that level. Even levels are left-to-right, odd levels are
+/// right-to-left. Normally, the minimum level is 0 (left-to-right), and the maximum level,
+/// according to [UAX #9 § 3.1.1 BD2][uax-bd2], is 125.


Incidentally, this being 125 does give us a bit to muck about with. I don't know of any use for that (maybe for whether rule L1 would apply to this?).

DJMcNab · 2026-07-25T00:47:55Z

+#[repr(transparent)]
+pub struct BidiLevel(u8);
+
+impl BidiLevel {


Does it make sense for max level to be stored here, either a u8 or BidiLevel constant?

That sounds sensible. I've added BidiLevel::MAX, but perhaps you have feedback on the naming.

In particular, it's not the greatest value BidiLevel itself can represent.

DJMcNab · 2026-07-25T00:49:42Z

+pub struct BidiLevel(u8);
+
+impl BidiLevel {
+    /// Construct a new bidi level.


I'd claim that we should debug assert that this is less than or equal to 125, but not blockingly so.

Perhaps that makes sense. This ties back into #710 (comment).

Deferring this for now.

DJMcNab · 2026-07-25T00:52:12Z

+// Safety: The struct is `repr(transparent)`, wrapping a `u8`.
+//
+// While generally BidiLevels have a maximum of 125, no value is unsound.
+unsafe impl Pod for BidiLevel {}
+
+// Safety: The struct is `repr(transparent)`, wrapping a `u8`.
+unsafe impl Zeroable for BidiLevel {}


We probably should have the usual safety tests for these. But also this is so simple it seems hard to imagine it going wrong! I'm not even sure what the tests would look like? Maybe even just that the size is 1 to force this to be revisited?

I've added the size test. I'm also not sure whether we can do better.

DJMcNab · 2026-07-25T00:59:40Z

    /// Returns `true` if the run has right-to-left directionality.
    pub fn is_rtl(&self) -> bool {
-        self.shaped.bidi_level & 1 != 0
+        self.shaped.bidi_level.is_rtl()
    }


I guess nominally it might be better to just return the bidi level here, but that's not done here to avoid breaking changes?

Yeah, I intentionally did not change the parley API here.

That said, we probably should break it. (But not in this PR.)

DJMcNab · 2026-07-25T01:02:17Z

    }

-    fn expect_bidi_embed_level_list(self, expected: Vec<u8>) -> Self {
+    fn expect_bidi_embed_level_list(self, expected: &[u8]) -> Self {


DJMcNab · 2026-07-25T01:05:05Z

+#[inline(always)]
+const fn next_odd(level: BidiLevel) -> BidiLevel {
+    BidiLevel::new((level.to_u8() + 1) | 1)
+}
+
+#[inline(always)]
+const fn next_even(level: BidiLevel) -> BidiLevel {
+    BidiLevel::new((level.to_u8() + 2) & !1)
+}


The lack of capping to 125 here would be my main hesitance, but I think on balance I agree (also with it being optional/up to your discretion)

tomcur · 2026-07-27T22:41:47Z

This probably should have changelog entries (what is parley_engine's changelog story?)

I've added a changelog entry for Parlance. I think we should write something in the changelog for parley_engine when we do the initial release (more or less describing the state it's in at that point).

tomcur commented Jul 24, 2026

View reviewed changes

tomcur force-pushed the push-wqvtnrxmlnkl branch from e483121 to 2d1f829 Compare July 24, 2026 14:56

nicoburns approved these changes Jul 24, 2026

View reviewed changes

DJMcNab approved these changes Jul 25, 2026

View reviewed changes

tomcur added 10 commits July 28, 2026 00:16

Changelog

efdf1d6

BidiLevel::MAX

0679441

Remove noisy comment

02a3651

Bytemuck in test

b128c3e

Test BidiLevel size

c2479b2

default_level return BidiLevel

dd81509

Add methods

c7923ac

Clippy

e41dc53

Better docs

4d40d99

More clippy

8f551ec

tomcur enabled auto-merge July 27, 2026 22:40

tomcur added this pull request to the merge queue Jul 27, 2026

Merged via the queue into linebender:main with commit f9b500a Jul 27, 2026
24 checks passed

tomcur deleted the push-wqvtnrxmlnkl branch July 27, 2026 22:52

Uh oh!

Conversation

tomcur commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomcur commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicoburns left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DJMcNab left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomcur Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomcur commented Jul 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tomcur commented Jul 24, 2026 •

edited

Loading

tomcur commented Jul 24, 2026 •

edited

Loading

DJMcNab left a comment •

edited

Loading

tomcur Jul 27, 2026 •

edited

Loading