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

Port font editor to SDL2 #59

Merged
merged 1 commit into from
Jan 6, 2025
Merged

Port font editor to SDL2 #59

merged 1 commit into from
Jan 6, 2025

Conversation

jouae
Copy link
Collaborator

@jouae jouae commented Oct 9, 2024

Replace X11 with SDL2 to improve the portability and cross-platform support

Migrated window creation from X11's XCreateWindow to SDL_CreateWindow.

Replaced X11 event handling with SDL2’s event loop (SDL_PollEvent) to capture input events such as keyboard and mouse interactions.

Updated rendering to use SDL_Renderer and SDL_Surface, replacing X11's rendering functions.

  • Modified some key event logic:
  1. SDLK_ESCAPE: ESC now exits the program.
  2. SDL_QUIT: Clicking the "X" on the window exits the program.
  • Unchanged key event logic:
  1. SDLK_q: Switches to the next font.
  • Features not fully implemented yet:
  1. SDLK_s, SDLK_u, SDLK_f, SDLK_d, SDLK_DOWN: Handling logic for stripe drawing operations.
  2. SDL_WINDOWEVENT, SDL_MOUSEBUTTONDOWN.

@jserv jserv requested a review from weihsinyeh October 9, 2024 12:20
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.h Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
jserv

This comment was marked as outdated.

@jserv
Copy link
Contributor

jserv commented Nov 23, 2024

You don't have to write the message Resolved in <commit> literally. Instead, simply click the button "Resolve conversation" once you have confirmed.

tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
@jouae jouae force-pushed the font-edit branch 2 times, most recently from 54d876b to dd01a82 Compare November 23, 2024 19:15
tools/font-edit/README.md Outdated Show resolved Hide resolved
tools/font-edit/README.md Outdated Show resolved Hide resolved
tools/font-edit/README.md Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@weihsinyeh weihsinyeh left a comment

Choose a reason for hiding this comment

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

How can I control the second control point, pt[1], when I press pt[2]? I can currently only control the first control point, pt[0].

@jouae
Copy link
Collaborator Author

jouae commented Nov 24, 2024

How can I control the second control point, pt[1], when I press pt[2]? I can currently only control the first control point, pt[0].

@weihsinyeh

Sorry, this function is not supported now.

I'm trying to figure out how to let the second control point be moved by user.

tools/font-edit/README.md Outdated Show resolved Hide resolved
{
char_t *c;

if (!init(argc, argv))
Copy link
Contributor

Choose a reason for hiding this comment

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

Dump the usage before calling exit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

+    char_t *c;
+
+   /*
+    * Use the following command to start twin-fedit:

Don't do that. Dump the message.

What kind of usage should I dump here?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can simply change the behavior of this font editor, allowing read a file from given filename (argv[1]). Thus, you can check argument count and the existence of file to proceed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

    if (argc < 2) {
        printf("Usage: %s <filename>\n", argv[0]);
        return 0;
    }

    FILE *file = fopen(argv[1], "r");
    if (!file) {
        printf("Failed to open file: %s\n", argv[1]);
        return 0;
    }
    if (!init(argc, argv))
        exit(1);
    while ((c = read_char(file)) && !exit_window) {
        play(c);
        print_char(c);
    }

    fclose(file);

Before initializing init(), use fopen to read the content of argv[1] and replace the previous method of reading strings from stdin in read_char().

I adopted this approach to make the file reading process clearer and improve the readability of the code. I'm not sure if there is a better way to improve it.

tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/README.md Outdated Show resolved Hide resolved
@jouae jouae force-pushed the font-edit branch 2 times, most recently from 3c63548 to fd57d77 Compare January 3, 2025 17:56
@jouae jouae requested a review from jserv January 4, 2025 04:59
@jserv jserv requested a review from weihsinyeh January 4, 2025 05:07
tools/font-edit/README.md Outdated Show resolved Hide resolved
tools/font-edit/README.md Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
tools/font-edit/twin-fedit.c Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Don't use backtick characters in git commit messages for the sake of terminal compatibility. Also, avoid unnecessary bullets.

@jouae jouae force-pushed the font-edit branch 2 times, most recently from 96170cf to 77b1872 Compare January 5, 2025 07:33
Comment on lines +481 to 485
int i = !!is_2nd_point;

push(c);
first->pt[i].x += dx;
first->pt[i].y += dy;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are two !.

Copy link
Contributor

Choose a reason for hiding this comment

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

The use of !! is a common trick to make sure the result will be either 0 or 1.

@jserv jserv requested a review from weihsinyeh January 5, 2025 13:29
@jouae jouae force-pushed the font-edit branch 2 times, most recently from ff93165 to cd0516c Compare January 5, 2025 19:28
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Append Close #7 at the end of git commit messages. Messages should use complete sentences and proper indentation.

Port font editor from X11 to SDL2, improving portability,
cross-platform support, and maintainability.

Window creation now uses SDL_CreateWindow, replacing X11 XCreateWindow.
Event handling is migrated to SDL2 SDL_PollEvent, streamlining
the capture of keyboard and mouse inputs.

Rendering is updated to leverage SDL_Renderer and SDL_Surface,
replacing X11 rendering functions.

Key event behaviors have been refined for better usability.
Pressing SDLK_ESCAPE or clicking the window close button (SDL_QUIT)
now exits the application.

To address a formatting issue with clang-format,
the function delete() has been renamed to delete_first_cmd(),
avoiding conflicts with the C++ keyword delete.

File reading has also been simplified using fopen for improved clarity.

The README has been expanded with a background on twin-fedit,
key bindings, a quick start guide,
and a demo GIF to help users get started.

Close sysprog21#7
@jouae
Copy link
Collaborator Author

jouae commented Jan 6, 2025

Append Close #7 at the end of git commit messages. Messages should use complete sentences and proper indentation.

@jserv

I adopted the 50/72 rule and refined the commit message to improve readability.

@jserv jserv merged commit 72b23f4 into sysprog21:main Jan 6, 2025
3 checks passed
@jserv
Copy link
Contributor

jserv commented Jan 6, 2025

Thank @jouae for contributing!

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.

3 participants