ziglibs / ansi-term

Zig library for dealing with ANSI terminals
MIT License
66 stars 7 forks source link

Proposal: Use integers for FontStyle #6

Closed joachimschmidt557 closed 4 years ago

joachimschmidt557 commented 4 years ago

Currently, FontStyle is represented as a (possibly packed after #5) struct of bools.

I think using integer types and bitwise operations, we could achieve better and possibly faster functionality. Consider the following simplified example:

const FontStyle = u3;

const default: FontStyle = 0b000;
const bold: FontStyle = 0b001;
const dim: FontStyle = 0b010;
const italic: FontStyle = 0b100;

Combining font styles can be done with bitwise or; checking for a style can be done with bitwise and.

cc @data-man

data-man commented 4 years ago

I think bitwise operations will decrease output's performance.

joachimschmidt557 commented 4 years ago

@data-man That may be right for checking a specific field of the FontStyle, i.e. checking whether font_style.bold == true. When using integers, we have an additional overhead of one bitwise operation in addition to the comparison.

But for other purposes, this has increased performance:

All operations are turned into O(1) operations instead of O(n) where n is the number of fields.

data-man commented 4 years ago

When using integers, we have an additional overhead of one bitwise operation in addition to the comparison.

And for convenience, some helpers will be needed: isBold/setBold, etc.

joachimschmidt557 commented 4 years ago

@data-man Yes, that would aid the usability of FontStyle if it would be represented as an integer. But those will be small functions which will be almost-surely inlined in a release build, so the overhead of an additional function call won't be there, only the overhead of the bitwise operation then.

I'll leave this proposal open until some benchmarks have been integrated into the library. From then on, we can test the performance of checking attributes/setting attributes/checking equality/subset of attributes/etc. And from these results, we can then proceed to select the appropriate representation of FontStyle

data-man commented 4 years ago

@joachimschmidt557 I think helpers for RGB type are more important. Maybe in new module. Rust's rgb24 as example.

joachimschmidt557 commented 4 years ago

@data-man If you want, feel free to implement that.

data-man commented 4 years ago
pub const FontStyle = packed struct {
...
const Self = @This();
...
pub fn asUInt(self: Self) u11 {
    return @bitCast(u11, self);
}
...

?

joachimschmidt557 commented 4 years ago

@data-man Yup, don't know why this hasn't occurred to me yet. This means we can get the best of both worlds. I guess we can close this issue now.