Fix alpha-channel check in IsImageOnlyTransparent and CopyRectangle rect math by ivandrofly · Pull Request #12579 · SubtitleEdit/subtitleedit

ivandrofly · 2026-07-17T17:27:19Z

Verified fixes for latent bitmap bugs, plus tests pinning them:

NikseBitmap.IsImageOnlyTransparent tested the blue byte instead of alpha (_bitmapData[i] in BGRA data) — an opaque blue-less image (black/yellow/red/green text) was reported as fully transparent, dropping line parts in the OCR image splitter. The identical bug in the OCR duplicate NikseBitmap2 is fixed in the same commit.
NikseBitmap.CopyRectangle(SKRect) passed width/height into SKRectI's (left, top, right, bottom) constructor — correct only when the section starts at the origin. The clamping statements in the SKRectI overload had the same Rectangle(x, y, w, h) → SKRectI(l, t, r, b) porting bug and are fixed as a follow-up commit, matching the SE 4 original's clamp-to-bitmap intent.
New unit tests pin the alpha-vs-blue regression (verified to fail against the old code) and the CopyRectangle offset/clamping math, for both NikseBitmap and NikseBitmap2.

The two FixInvalidItalicTags commits from the original version of this PR were dropped after review — see comment below.

All 508 libse and 14 libuilogic tests pass.

🤖 Generated with Claude Code

Copilot · 2026-07-18T07:26:05Z

@niksedk Unfortunately, I hit an error while trying to use the custom Copilot setup steps configured for this repository and had to close this PR. The error I am seeing is:

no `copilot-setup-steps` job found in your `copilot-setup-steps.yml` workflow file. Please ensure you have a single job named `copilot-setup-steps`. For more details, see https://gh.io/copilot/actions-setup-steps

Once you or someone with the necessary access fixes the problem, please unassign and then reassign issue #0 to me and I'll retry. Thanks!

Copilot

Pull request overview

This PR fixes several small but correctness-impacting issues in bitmap handling and HTML italic tag cleanup, improving OCR/image processing reliability and tag normalization.

Changes:

Fix alpha-channel detection in IsImageOnlyTransparent for both NikseBitmap and the OCR copy NikseBitmap2.
Correct CopyRectangle(SKRect) to construct SKRectI using Right/Bottom rather than Width/Height.
Fix duplicated EndsWith(...) conditions in HtmlUtil.FixInvalidItalicTags for 2-line edge cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/libuilogic/Ocr/NikseBitmap2.cs	Fix alpha-byte indexing when checking transparency.
src/libse/Common/NikseBitmap.cs	Fix `CopyRectangle(SKRect)` rect construction and alpha-byte indexing for transparency checks.
src/libse/Common/HtmlUtil.cs	Fix incorrect line index usage in italic tag repair conditions.

+            var rect = new SKRectI((int)section.Left, (int)section.Top, (int)section.Right, (int)section.Bottom);
            return CopyRectangle(rect);


niksedk · 2026-07-18T08:46:21Z

AI-generated bug fix pull requests

For future AI-generated bug fix PRs, please limit each pull request to 1–3 related fixes (depending on their size and complexity). Smaller, focused PRs are much easier to review, test, and merge.

Please also include a brief manual comment describing how you personally verified each fix. AI-generated code can contain mistakes or unintended side effects, so every submitted change should be tested and validated by the contributor before opening the PR.

The loop tested _bitmapData[i] (blue in BGRA) instead of the alpha byte at i+3, so an opaque all-black image was reported as fully transparent. Fix both the libse and OCR-duplicate implementations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The SKRectI constructor takes (left, top, right, bottom), but the overload passed Width/Height as the right/bottom coordinates, which is only correct when Left and Top are both 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The clamp statements had the same Rectangle(x, y, width, height) to SKRectI(left, top, right, bottom) porting bug as the SKRect overload: they passed width/height values as right/bottom. Clamp bottom to Height and right to Width directly, matching the SE 4 original's intent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Pin the alpha-vs-blue regression (opaque black must not be "only transparent") for both NikseBitmap and the OCR NikseBitmap2, and the CopyRectangle offset/clamping rect math. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

niksedk · 2026-07-18T09:02:28Z

Thanks for the PR! The two bitmap fixes are verified and merged (with the SKRectI clamp statements completed, since they had the same (x, y, w, h) → (l, t, r, b) porting issue, plus regression tests).

The two FixInvalidItalicTags commits were dropped after careful review:

The "duplicated" lines[1].EndsWith condition is identical in SE 4 (se4-legacy branch) — it's decade-old shipped behavior, not an SE 5 porting typo.
Behavioral testing showed the change is a regression: the loose condition is load-bearing. Inputs like Hello\naWorld or Hellox\nWorld are repaired to valid italics today (Hello + aWorld), but with the tightened condition they fall through unfixed — the function returns the invalid tags unchanged, as there is no downstream fallback for these shapes.

🤖 Generated with Claude Code

…aired The two-line branches for "only four tags" / "only four tags" had an asymmetric condition (lines[1].EndsWith checked twice, lines[0]'s end never) - identical in SE 4, and load-bearing: shapes it matched were repaired to whole-line italics, but near-miss shapes (e.g. second line not starting with the tag, or all four tags on the first line) fell through and the function returned the invalid tags unchanged. Rather than tightening the condition (which would repair fewer shapes - see PR SubtitleEdit#12579 review), generalize it: under the count guard the stray tags carry no pairing information, so strip them from each line that contains one and wrap that line in proper ...; tag-less lines are kept as-is. Output is byte-identical for every previously matched shape, and the fall-through shapes now come out as valid italics. Tests pin both the long-standing shapes and the newly repaired ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

niksedk requested a review from Copilot July 18, 2026 07:25

Copilot started reviewing on behalf of niksedk July 18, 2026 07:26 View session

Copilot AI reviewed Jul 18, 2026

View reviewed changes

Comment thread src/libse/Common/NikseBitmap.cs

Comment on lines +1181 to 1182

var rect = new SKRectI((int)section.Left, (int)section.Top, (int)section.Right, (int)section.Bottom);

return CopyRectangle(rect);

ivandrofly and others added 4 commits July 18, 2026 10:59

Add tests for IsImageOnlyTransparent and CopyRectangle

b42230d

Pin the alpha-vs-blue regression (opaque black must not be "only transparent") for both NikseBitmap and the OCR NikseBitmap2, and the CopyRectangle offset/clamping rect math. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

niksedk force-pushed the fix/bitmap-latent-fixes branch from 96b431b to b42230d Compare July 18, 2026 09:02

niksedk changed the title ~~Fix alpha-channel check in IsImageOnlyTransparent, CopyRectangle rect math, and FixInvalidItalicTags conditions~~ Fix alpha-channel check in IsImageOnlyTransparent and CopyRectangle rect math Jul 18, 2026

niksedk merged commit 92f879d into SubtitleEdit:main Jul 18, 2026
1 of 2 checks passed

niksedk mentioned this pull request Jul 18, 2026

Fix FixInvalidItalicTags leaving some two-line stray-tag shapes unrepaired #12593

Merged

ivandrofly deleted the fix/bitmap-latent-fixes branch July 18, 2026 09:55

niksedk mentioned this pull request Jul 18, 2026

Update change log + version to 5.1.0-rc8 #12600

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix alpha-channel check in IsImageOnlyTransparent and CopyRectangle rect math#12579

Fix alpha-channel check in IsImageOnlyTransparent and CopyRectangle rect math#12579
niksedk merged 4 commits into
SubtitleEdit:mainfrom
ivandrofly:fix/bitmap-latent-fixes

ivandrofly commented Jul 17, 2026 •

edited by niksedk

Loading

Uh oh!

Copilot AI commented Jul 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

niksedk commented Jul 18, 2026

Uh oh!

niksedk commented Jul 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		var rect = new SKRectI((int)section.Left, (int)section.Top, (int)section.Right, (int)section.Bottom);
		return CopyRectangle(rect);

Uh oh!

Conversation

ivandrofly commented Jul 17, 2026 • edited by niksedk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI commented Jul 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

niksedk commented Jul 18, 2026

AI-generated bug fix pull requests

Uh oh!

niksedk commented Jul 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ivandrofly commented Jul 17, 2026 •

edited by niksedk

Loading