优化 APNG 支持 #4230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Draft

Glavo wants to merge 17 commits into HMCL-dev:main

from Glavo:apng

Draft

优化 APNG 支持 #4230

Glavo wants to merge 17 commits into HMCL-dev:main from Glavo:apng

Conversation

@Glavo

Copy link

Member

@Glavo Glavo commented Aug 9, 2025

目前还无法工作,待修复。

Glavo added 8 commits

August 9, 2025 15:53

@Glavo


 Create BgraPreCanvas

aa8f0e4

@Glavo


 update

88db57b

@Glavo


 update BgraPreCanvas

8b3605d

@Glavo


 update

e03d855

@Glavo


 update

5830d7c

@Glavo


 update

a2ff22b

@Glavo


 update

dae4d3c

@Glavo


 update

eb51449

@Glavo Glavo requested a review from Copilot

August 9, 2025 13:15

Copilot

This comment was marked as outdated.

@burningtnt burningtnt mentioned this pull request

Aug 9, 2025

PR Collection 开发进展 burningtnt/HMCL#9

Closed

Glavo and others added 6 commits

August 14, 2025 21:04

@Glavo @Copilot


 Update HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPr...

fe0dd17

...eCanvas.java
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@Glavo @Copilot


 Update HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPr...

d0e9fcf

...eCanvas.java
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@Glavo @Copilot


 Update HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPr...

1c7d44e

...eCanvas.java
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@Glavo @Copilot


 Update HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPr...

d805558

...eCanvas.java
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@Glavo @Copilot


 Update HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPr...

ec5ac5a

...eAnimationImage.java
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@Glavo @Copilot


 Update HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPr...

46f4f8f

...eCanvas.java
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

@Glavo Glavo requested a review from Copilot

August 14, 2025 13:18

@Glavo


 update

cc86e07

Copilot

This comment was marked as outdated.

@Glavo Glavo requested a review from Copilot

August 14, 2025 13:22

Copilot

Copilot AI reviewed

Aug 14, 2025

View reviewed changes

Copy link

Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the APNG (Animated PNG) support system to optimize image handling for animated images. The changes replace the old interface-based animation system with a new class-based approach and introduce specialized BGRA pre-multiplied pixel handling for better animation performance.

Key changes:

Converts AnimationImage from interface to abstract class with built-in animation timeline management
Introduces new BGRA pre-multiplied pixel format classes for more efficient animation frame handling
Replaces the old APNG processing logic in ImageUtils with a new director-based approach

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`AnimationImage.java`	Converted from interface to abstract class with animation timeline management
`AnimationFrame.java`	New interface defining animation frame duration contract
`BgraPreCanvas.java`	New canvas class for BGRA pre-multiplied pixel operations
`BgraPreFrame.java`	New animation frame implementation extending BgraPreCanvas
`BgraPreAnimationImage.java`	New animation image implementation using BGRA pre-multiplied format
`AnimationImageImpl.java`	Refactored to extend new AnimationImage base class
`BgraPreBitmapDirector.java`	New director class for processing APNG files into BGRA format
`Argb8888Director.java`	Added exception handling to receiveFrameImage method
`ImageUtils.java`	Simplified APNG loading by replacing complex logic with director pattern

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPreCanvas.java

import java.util.Arrays;

import java.util.Objects;

public class BgraPreCanvas {

Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The argbToPre method lacks documentation explaining what 'pre' means and the purpose of the alpha premultiplication calculation.

Suggested change

public class BgraPreCanvas {

/**

* Converts an ARGB color value to its premultiplied alpha ("pre") form.

* <p>

* In premultiplied alpha format, the red, green, and blue color components are

* each multiplied by the alpha component and divided by 255. This is commonly

* used in image compositing to simplify blending calculations and improve performance.

* <p>

* For example, if the original color is (A, R, G, B), the premultiplied color is

* (A, R * A / 255, G * A / 255, B * A / 255).

* @param argb the original ARGB color value (with 8 bits per channel)

* @return the color value in premultiplied alpha format

Copilot uses AI. Check for mistakes.

HMCL/src/main/java/org/jackhuang/hmcl/ui/image/internal/BgraPreCanvas.java

ByteArray.setIntLE(pixels, targetIndex, resultArgbPre);

}

Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The toString() method override simply calls super.toString() without adding any value. This override should either provide meaningful information about the canvas state or be removed.

Copilot uses AI. Check for mistakes.

HMCL/src/main/java/org/jackhuang/hmcl/ui/image/apng/argb8888/BgraPreBitmapDirector.java

@Override

public Argb8888ScanlineProcessor receiveFrameControl(PngFrameControl control) {

//throw new IllegalStateException("TODO up to here");

//return null;

Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented-out code should be removed. These TODO comments and dead code lines make the codebase harder to maintain.

Suggested change

//return null;

Copilot uses AI. Check for mistakes.

HMCL/src/main/java/org/jackhuang/hmcl/ui/image/apng/argb8888/BgraPreBitmapDirector.java

//return null;

currentFrame = control;

//System.out.println("Frame: "+control);

Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug print statement should be removed from production code.

Suggested change

//System.out.println("Frame: "+control);

Copilot uses AI. Check for mistakes.

HMCL/src/main/java/org/jackhuang/hmcl/ui/image/apng/argb8888/BgraPreBitmapDirector.java

@Override

public void receiveDefaultImage(Argb8888Bitmap bitmap) {

// this.bitmapSequence.receiveDefaultImage(bitmap);

Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented-out code should be removed to keep the codebase clean.

Suggested change

// this.bitmapSequence.receiveDefaultImage(bitmap);

Copilot uses AI. Check for mistakes.

HMCL/src/main/java/org/jackhuang/hmcl/ui/image/apng/argb8888/BgraPreBitmapDirector.java

//System.out.println("Frame: "+control);

return scanlineProcessor.cloneWithNewBitmap(header.adjustFor(control)); // TODO: is this going to be a problem?

Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO comment indicates uncertainty about potential issues. This should be investigated and resolved or properly documented if the concern is valid.

Suggested change

returnscanlineProcessor.cloneWithNewBitmap(header.adjustFor(control)); // TODO: is this going to be a problem?

// It is safe to clone the scanline processor with a new bitmap for each frame,

// as cloneWithNewBitmap creates an independent processor instance for the frame's region.

return scanlineProcessor.cloneWithNewBitmap(header.adjustFor(control));

Copilot uses AI. Check for mistakes.

HMCL/src/main/java/org/jackhuang/hmcl/ui/image/apng/argb8888/BgraPreBitmapDirector.java

@Override

public Image getResult() {

if (doScale) {

throw new UnsupportedOperationException("TODO"); // TODO

Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scaling functionality is not implemented but the method can be called when doScale is true. This will cause runtime failures when users request scaled images.

Suggested change

thrownewUnsupportedOperationException("TODO"); // TODO

// Scale each frame to the target size

List<BgraPreFrame> scaledFrames = new ArrayList<>(animationFrames.size());

for (BgraPreFrame frame : animationFrames) {

scaledFrames.add(scaleFrame(frame, targetWidth, targetHeight));

}

int cycleCount;

if (animationControl != null) {

cycleCount = animationControl.numPlays;

if (cycleCount == 0)

cycleCount = Timeline.INDEFINITE;

} else

cycleCount = Timeline.INDEFINITE;

return new BgraPreAnimationImage(targetWidth, targetHeight, cycleCount, List.copyOf(scaledFrames));

Copilot uses AI. Check for mistakes.

Glavo added 2 commits

August 14, 2025 21:28

@Glavo

fix

6651dcb

@Glavo


 Merge remote-tracking branch 'upstream/main' into apng

805dd2b

Labels

None yet

1 participant

@Glavo

优化 APNG 支持 #4230

Are you sure you want to change the base?

优化 APNG 支持 #4230

Uh oh!

Conversation

@Glavo Glavo commented Aug 9, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant