How does the library deal with higher embed level than 125?

[asibahi]

Hello,

I am not familiar with C, and I have been having trouble reading and following the code.

How does the library deal with higher embed level than 125? The rust library, unicode-bidi simply panics. As fast as I can tell, this is the relevant part of the codebase:

static SBLevel DetermineParagraphLevel(BidiChainRef chain, SBLevel baseLevel)
{
    if (baseLevel >= SBLevelMax) {
        return DetermineBaseLevel(chain, chain->roller, chain->roller,
                                  (baseLevel != SBLevelDefaultRTL ? 0 : 1),
                                  SBFalse);
    }

    return baseLevel;
}

But I do not really understand why the DetermineBaseLevel function is getting called, as since the loop starts and ends with the same link it should skip the loop and return the defaultValue? this doesn't seem right. Below is the body of the function.

static SBLevel
DetermineBaseLevel(
	BidiChainRef chain, 
	BidiLink skipLink, 
	BidiLink breakLink,
	SBLevel defaultLevel,
	SBBoolean isIsolate
) {
    BidiLink link = skipLink;

    /* Rules P2, P3 */
    while ((link = BidiChainGetNext(chain, link)) != breakLink) {
        SBBidiType type = BidiChainGetType(chain, link);

        switch (type) {
        case SBBidiTypeL:
            return 0;

        case SBBidiTypeAL:
        case SBBidiTypeR:
            return 1;

        case SBBidiTypeLRI:
        case SBBidiTypeRLI:
        case SBBidiTypeFSI:
            link = SkipIsolatingRun(chain, link, breakLink);
            if (link == BidiLinkNone) {
                goto Default;
            }
            break;

        case SBBidiTypePDI:
            if (isIsolate) {
                /*
                 * In case of isolating run, the PDI will be the last code point.
                 * NOTE:
                 *      The inner isolating runs will be skipped by the case above this one.
                 */
                goto Default;
            }
            break;
        }
    }

Default:
    return defaultLevel;
}

what am I missing here? Is the overflow dealt with in some other place?

(slight aside, why the goto? why not just return defaultLevel immediately?)

[behdad]

How does the library deal with higher embed level than 125? The rust library, unicode-bidi simply panics.

I'm not sure what the question is. Do you mean if there's enough embedding chars that the level hits the max? Panic sounds very wrong...

(slight aside, why the goto? why not just return defaultLevel immediately?)

That's just to reduce copy/paste.

[asibahi]

How does the library deal with higher embed level than 125? The rust library, unicode-bidi simply panics.

I'm not sure what the question is. Do you mean if there's enough embedding chars that the level hits the max?

yes, or exceeds it

[behdad]

panic is definitely bad. The algorithm specifies that. Off the top of my head, (from implementing it in FriBidi 25 years ago), you start ignoring new embedding chars while counting the number of them, and count down every time you see a POP DIRECTIONAL FLAG, the level reduces to under the max where normal processing continues. This is a simplification from before isolates were introduced. The Unicode test file has tests for this.

[behdad]

https://github.com/fribidi/fribidi/blob/b28f43bd3e8e31a5967830f721bab218c1aa114c/lib/fribidi-bidi.c#L300-L368

[asibahi]

As far as I can tell we are talking about different parts of the algorithm/codebases.

for explicit levels, unicode-bidi deals with it the same way SheenBidi does, as far as I can tell, with two overflow counters and everything.

However, for implicit levels, I1 and I2, this is the relevant code from unicode-bidi:

pub fn resolve_levels(processing_classes: &[BidiClass], levels: &mut [Level]) -> Level {
    let mut max_level = Level::ltr();
    assert_eq!(processing_classes.len(), levels.len());
    for i in 0..levels.len() {
        match (levels[i].is_rtl(), processing_classes[i]) {
            (false, AN) | (false, EN) => levels[i].raise(2).expect("Level number error"),
            (false, R) | (true, L) | (true, EN) | (true, AN) => {
                levels[i].raise(1).expect("Level number error")
            }
            // <https://www.unicode.org/reports/tr9/#Retaining_Explicit_Formatting_Characters> handled here
            (_, _) => {}
        }
        max_level = max(max_level, levels[i]);
    }

    max_level
}

While in SheenBidi this is the relevant function:

static void ResolveImplicitLevels(IsolatingRunRef isolatingRun)
{
    BidiChainRef chain = isolatingRun->bidiChain;
    BidiLink roller = chain->roller;
    BidiLink link;

    SBLevel runLevel = isolatingRun->baseLevelRun->level;
    
    if ((runLevel & 1) == 0) {
        BidiChainForEach(chain, roller, link) {
            SBBidiType type = BidiChainGetType(chain, link);
            SBLevel level = BidiChainGetLevel(chain, link);
            
            SBAssert(SBBidiTypeIsStrongOrNumber(type));
            
            /* Rule I1 */
            if (type == SBBidiTypeR) {
                BidiChainSetLevel(chain, link, level + 1);
            } else if (type != SBBidiTypeL) {
                BidiChainSetLevel(chain, link, level + 2);
            }
        }
    } else {
        BidiChainForEach(chain, roller, link) {
            SBBidiType type = BidiChainGetType(chain, link);
            SBLevel level = BidiChainGetLevel(chain, link);
            
            SBAssert(SBBidiTypeIsStrongOrNumber(type));
            
            /* Rule I2 */
            if (type != SBBidiTypeR) {
                BidiChainSetLevel(chain, link, level + 1);
            }
        }
    }
}

Which seems to behave similarly to the comment you linked from FriBidi. I am entirely clear on either section of the code, tbh, but you can clearly see the rust code panicking with expect. Does the C code simply not raise it, or what? (is it like a saturating add?)

Edit: the panics don't hit in the unicode conformance test.

[behdad]

If your question is whether the resolved levels can go above the max (by one or two), that is correct. That's why the max is 125, not 127.