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 truecolor support, thanks @deathlyfrantic #11

Closed
wants to merge 1 commit into from
Closed

Add truecolor support, thanks @deathlyfrantic #11

wants to merge 1 commit into from

Conversation

amirouche
Copy link

... based on a patch by Zandr Martin.

... based on a patch by Zandr Martin.
@amirouche
Copy link
Author

related issue: #2

@amirouche
Copy link
Author

By the way thanks for taking on the maintenance of this project. I like the move to a makefile (even if am not an expert of m4 macros!).

@rofl0r
Copy link
Contributor

rofl0r commented May 1, 2021

I like the move to a makefile (even if am not an expert of m4 macros!).

what does a Makefile have to do with m4 macros ?

@amirouche
Copy link
Author

I like the move to a makefile (even if am not an expert of m4 macros!).

what does a Makefile have to do with m4 macros ?

Sorry, It seems there is no m4 macro in the makefile. But I do not know what the $(foobar ...) does, e.g.:

termbox_demos:=$(patsubst demo/%.c,demo/%,$(wildcard demo/*.c))

@rofl0r
Copy link
Contributor

rofl0r commented May 1, 2021

it's calling gnu make functions. if you search for "GNU make patsubst" you will find the appropriate manual.

@adsr
Copy link
Contributor

adsr commented May 25, 2021

Thanks @amirouche. Would you like to try writing a test for this? I just setup an integration test suite https://github.com/termbox/termbox/tree/master/tests

@stacksmith
Copy link

This will break existing bindings and require rewriting bindings and code.

The gain is marginal. While I do appreciate full color, this is a console text library, very few people can tell between 16-bit and 24-bit color when it comes to finer shades of characters or backgrounds. It's a different story for images (even then dithered 16-bit color is largely indistinguishable from 24-bit at today's resolutions).

TB_CELL is a key structure. Current configuration - 32-bit character and two 16-bit color designators has a 64-bit footprint and is easily passed around in a register and updated atomically. With two 32-bit color fields it is 96 bits and requires two separate moves.

I strongly advise against this breaking change in the main branch.

@amirouche
Copy link
Author

Let me know. @deathlyfrantic what do you think?

If the problem is only bindings, it is better to do the change sooner than later.

@deathlyfrantic
Copy link

Sorry, I have no dog in this fight. I haven't used termbox, or any programs that use termbox, in many years.

@adsr
Copy link
Contributor

adsr commented Sep 3, 2021

Hi all. I've been working on termbox2 which I believe is relevant to this conversation. I'll be soliciting code reviews soon, but for now I will summarize.

I propose termbox2 as mostly a cleanup of v1 with a few key differences:

  1. All libc calls are error-checked
  2. (Relevant to this conversation) The library is designed as a single-file header library

I predict item 1 will be uncontroversial. Item 2 will solve issues like 24-bit color for us. Some users want a tight tb_cell struct. Some want 24-bit color (#define TB_OPT_24BIT_COLOR). Both are happy. Of course I am not calling for an ifdef for every feature -- just for things that change key types or interfaces. Another one might be for extended grapheme cluster support which also requires adding some fields to tb_cell.

Feedback welcome.

@amirouche
Copy link
Author

amirouche commented Sep 3, 2021

I am not familiar with single-file header, what does it change when called from a C FFI ?

@adsr
Copy link
Contributor

adsr commented Sep 4, 2021

@amirouche No change. You can still compile it as a shared library and access via FFI the same way you would now. In fact that's exactly how termbox2 tests are written (via PHP FFI). But being a single file, it also makes it easy to copy-paste or #include into a project and configure as desired via #defines.

@amirouche
Copy link
Author

I close this PR waiting on termbox2

@amirouche amirouche closed this Sep 14, 2021
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.

5 participants