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

Support Unicode 16 octant characters #6502

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

eschnett
Copy link

Closes #6494.

@bew
Copy link
Contributor

bew commented Dec 20, 2024

Hello o/

Your PR is currently adding more than a thousand lines to draw a few squares, I feel like there's room for improvement and this can be much reduced 🤔

For example when I worked on adding Braille characters (which are also in a 2x4 grid), I managed to implement the drawing part in ~60 lines:

BlockKey::Braille(dots_pattern) => {
// `dots_pattern` is a byte whose bits corresponds to dots
// on a 2 by 4 dots-grid.
// The position of a dot for a bit position (1-indexed) is as follow:
// 1 4 |
// 2 5 |<- These 3 lines are filled first (for the first 64 symbols)
// 3 6 |
// 7 8 <- This last line is filled last (for the remaining 192 symbols)
//
// NOTE: for simplicity & performance reasons, a dot is a square not a circle.
let dot_area_width = metrics.cell_size.width as f32 / 2.;
let dot_area_height = metrics.cell_size.height as f32 / 4.;
let square_length = dot_area_width / 2.;
let topleft_offset_x = dot_area_width / 2. - square_length / 2.;
let topleft_offset_y = dot_area_height / 2. - square_length / 2.;
let (width, height) = buffer.image_dimensions();
let mut pixmap = PixmapMut::from_bytes(
buffer.pixel_data_slice_mut(),
width as u32,
height as u32,
)
.expect("make pixmap from existing bitmap");
let mut paint = Paint::default();
paint.set_color(tiny_skia::Color::WHITE);
paint.force_hq_pipeline = true;
paint.anti_alias = true;
let identity = Transform::identity();
const BIT_MASK_AND_DOT_POSITION: [(u8, f32, f32); 8] = [
(1 << 0, 0., 0.),
(1 << 1, 0., 1.),
(1 << 2, 0., 2.),
(1 << 3, 1., 0.),
(1 << 4, 1., 1.),
(1 << 5, 1., 2.),
(1 << 6, 0., 3.),
(1 << 7, 1., 3.),
];
for (bit_mask, dot_pos_x, dot_pos_y) in &BIT_MASK_AND_DOT_POSITION {
if dots_pattern & bit_mask == 0 {
// Bit for this dot position is not set
continue;
}
let topleft_x = (*dot_pos_x) * dot_area_width + topleft_offset_x;
let topleft_y = (*dot_pos_y) * dot_area_height + topleft_offset_y;
let path = PathBuilder::from_rect(
tiny_skia::Rect::from_xywh(
topleft_x,
topleft_y,
square_length,
square_length,
)
.expect("valid rect"),
);
pixmap.fill_path(&path, &paint, FillRule::Winding, identity, None);
}
}

The idea for braille chars was that I noticed that the sequencing of chars was mostly following a binary counting logic, which I then translated into drawing the correct dots.

I didn't analyzed how the sequencing of blocks is done, but there's probably a way to reduce it in a similar logic once you find a logic in the sequence 🤔

@eschnett
Copy link
Author

Unfortunately there is no logic in the sequence. The existing half and quad characters are reused, and some other characters are taken from other ranges. Only the remaining ones are encoded in a sequence. It's not a nice as the Braille characters. The mapping from octants (represented as binary) to code points starts with [0020, 1CEA8, 1CEAB, 1FB82, 1CD00, 2598, 1CD01, 1CD02, ...]

I can change the code to match on ranges of code points, and within that range look up a binary value in an array, and then map the binary bits to octants. It would still be longer than the Braille code.

@eschnett
Copy link
Author

Alternatively I could add a new field to BlockKey, and store the octant pattern as u8 value there instead of as list of blocks. (Such a representation would also work for sextants.) The mapping from code point to octant pattern would still required either a lot of code or an array lookup.

@eschnett
Copy link
Author

I have replaced the match statements by array lookups. The code has become smaller, but as I mentioned above, there is no pattern (unlike for the Braille characters). I've taken the liberty to apply the same change to the sextant blocks as well.

// ├───┼───┤
// │ 4 │ 5 │
// ╰───┴───╯
const SEXTANT_PATTERNS: [u8; 60] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello! Thanks for making the changes, this sextant sequences looks almost perfectly like a continuous sequence, only skipping 0b101010.
In my opinion I'd just impl this as a perfect sequence, implementing even the one that is not defined in spec 🤔

I'm not the one who's going to review this in the end, that's @wez' thing, I'm just a 'pre-processor' here.

nice job anyway 👍

Copy link
Author

Choose a reason for hiding this comment

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

This array is the mapping from code points to sextant patterns. It's fixed by the standard – I couldn't just add the four missing ones.

The code that takes the bit pattern and draws the sextants is generic. It works for all 64 possible patterns although only 60 of them will ever be encountered.

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.

Octants in Unicode 16
2 participants