Skip to content
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

Add PolyBuffer and use it to handle 8/16-bit conversion in LXLayeredComponent. #3

Open
wants to merge 1 commit into
base: ping/lxcolor16
Choose a base branch
from

Conversation

zestyping
Copy link
Collaborator

@zestyping zestyping commented Apr 26, 2018

It's a major goal of this project to support existing 8-bit effects and patterns unchanged alongside new 16-bit effects and patterns, because it isn't practical to have to port all effects and patterns to run in 16-bit colour in one huge swoop.

A good place to start reviewing is the new class PolyBuffer.

This PR introduces PolyBuffer, a useful tool for handling 8-bit/16-bit conversions. It encapsulates a pair of colour buffers—an 8-bit colour buffer and a 16-bit colour buffer—and converts between them as needed, whenever anyone requests the data in a different colour depth than what was last written. It keeps track of which buffers are fresh or stale, so that it can automatically convert the data as needed without wasting time on unnecessary conversions.

LXLayeredComponent is the base for LXEffect and LXPattern. Its colour buffer is now a PolyBuffer, and it exposes its two arrays via its typeless getArray() method.

An added twist is that LXLayeredComponent supports two modes of operation already: it can either allocate and own its own buffer (which is what patterns do), or it can write to and modify an externally provided LXBuffer object (which is what effects do). The Buffered marker interface is used to select between these two modes.

@zestyping zestyping force-pushed the ping/hybrid-buffer branch from aef13cc to 22b6d72 Compare April 30, 2018 10:26
@zestyping zestyping changed the title Add HybridBuffer and use it to handle 8/16-bit conversion in LXLayeredComponent. Add PolyBuffer and use it to handle 8/16-bit conversion in LXLayeredComponent. Apr 30, 2018
Copy link
Member

@kylefleming kylefleming left a comment

Choose a reason for hiding this comment

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

We should consider separating the color space with the color depth. For example, an additive blend would request that the color buffers be in linear color space but wouldn't care the color depth.

This becomes especially important in java where we have to specify the type (int vs long) but could use the same variable for both linear 48-bit and gamma compressed 48-bit. (for example, if statements checking for color depth in order to declare the correct type only needs to check the color depth)

public enum Space {RGB8, RGB16};

private LX lx = null;
private Map<Space, Buffer> buffers = new EnumMap<>(Space.class);
Copy link
Member

Choose a reason for hiding this comment

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

I would consider having something akin to IntBuffer and LongBuffer as subclasses of Buffer and then having public IntBuffer getIntBuffer() or public IntBuffer getIntBuffer(Space space) (as well as the long versions). Then in those methods it could double-check that the Buffer is of the correct type before casting to the buffer subclass in order to call the appropriate method on the buffer subclass. The purpose of this would be to get rid of unsafe casts and to avoid returning Object from methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants