Skip to content

Conversation

dario-loi
Copy link
Contributor

What's in the Pull Request

A general overview of the changes can be found in the first commit message, which I'll paste down below

Commit message

Provided a working CMakeLists.txt file to build all of the examples,
also fixed a bunch of bugs (some fixes also depend on personal
preference and should be reviewed by maintainers):

  1. Switched fbgl_t types from size_t to uint32_t as there is no way a
    screen resolution will be more than 2^32, this way we save 4 bytes
    and we have faster comparisons
  2. Used int32_t instead of int for better clarity
  3. fbgl_point now uses int32_t instead of uint32_t, no more sign
    comparison warnings (for loops that worked with fbgl_point were
    already working with int32_t in practice)
  4. line.c now uses usleep(1000) to wait inbetween individual line draws
    instead of an empty for loop, as that was optimized away in release
    builds and led to all the animation being played instantly
  5. removed the void fbgl_set_bg() forward declaration as it was
    incorrect w.r.t the actual function definition
  6. fbgl_draw_texture specifies a const pointer to the texture resource,
    possibly allowing for compiler optimizations
  7. fbgl_set_bg and fbgl_put_pixel now use #ifdef DEBUG to compile away
    checks in release builds, for faster performance (This has been adapted to FBGL_VALIDATE_PUT_PIXEL, according to master, I still suggest DEBUG so it can be done automatically by CMake)
  8. various const-correctness improvements
  9. all required UNIX headers have been imported so that
    fbgl_check_esc_key does not give any more warnings

All examples have been tested (on my local machine, so take this with a
grain of salt)

Additional Changes

The example directory has been renamed into examples, files that have not been affected past that in this PR will show in the "files changed" tab as if the whole file was rewritten (this is how git sees renaming)

Formatting has been applied in line with the .clang-format so there should not be any more useless diffs from now on.

Provided a working CMakeLists.txt file to build all of the examples,
also fixed a bunch of bugs (some fixes also depend on personal
preference and should be reviewed by maintainers):

1. Switched fbgl_t types from size_t to uint32_t as there is no way a
   screen resolution will be more than 2^32, this way we save 4 bytes
   and we have faster comparisons
2. Used int32_t instead of int for better clarity
3. fbgl_point now uses int32_t instead of uint32_t, no more sign
   comparison warnings (for loops that worked with fbgl_point were
   already working with int32_t in practice)
4. line.c now uses usleep(1000) to wait inbetween individual line draws
   instead of an empty for loop, as that was optimized away in release
   builds and led to all the animation being played instantly
5. removed the void fbgl_set_bg() forward declaration as it was
   incorrect w.r.t the actual function definition
6. fbgl_draw_texture specifies a const pointer to the texture resource,
   possibly allowing for compiler optimizations
7. fbgl_set_bg and fbgl_put_pixel now use #ifdef DEBUG to compile away
   checks in release builds, for faster performance
8. various const-correctness improvements
9. all required UNIX headers have been imported so that
   fbgl_check_esc_key does not give any more warnings

All examples have been tested (on my local machine, so take this with a
grain of salt)
@dario-loi
Copy link
Contributor Author

I promise I will split future features into smaller PRs/commits :)

Copy link
Owner

Choose a reason for hiding this comment

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

I liked how well it complied with the standards, great job. Thank you 🙏

@lvntky
Copy link
Owner

lvntky commented Nov 24, 2024

Awesome commit, thank you

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