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

core: separate work with the x extensions #1053

Merged
merged 7 commits into from
Nov 11, 2024

Conversation

absolutelynothelix
Copy link
Collaborator

@absolutelynothelix absolutelynothelix commented Apr 23, 2023

  1. a todo is addressed:

    picom/src/x.c

    Line 67 in 1648673

    // TODO(yshui) separate error code out from session_t
  2. unused information about the x extensions (base event and error numbers, major opcode) is not stored anymore;
  3. information about the x extensions (presence, base event and error numbers) is now stored in a single, separate place - a structure called x_extensions that lives in the x_connection structure, so the session structure is less polluted. it's ordered and clean so it's clear how to add and/or remove the x extensions;
  4. the x extensions are now initialized in a single, separate place - a function called x_init_extensions that is called right after the x_init_connection function, so the session_init function is less polluted. it's ordered and clean so it's clear how to add and/or remove the x extensions. also it separates initialization from work (only initialization happens there, all the work left in the session_init function), has comments (when a particular x extension initialization happens, why we require particular versions of particular x extensions and why we sometimes negotiate versions of x extensions just to discard this information) and has a better error handling (when a required x extension is missing or something goes wrong it at least tries to free what's already allocated instead of immediately dying using exit(1) and leaking a bunch of stuff);
  5. --diagnostics reports presence of all the optional x extensions in the same way now;
  6. the win_update_bounding_shape function doesn't require shape_exists that indicates the x shape extension presence, it determines it itself using c;
  7. the x_error_code_to_string function doesn't use ps_g anymore, it uses c instead;
  8. the x extensions used by picomling are now properly initialized and a picomling won't born if one of them is missing.

@absolutelynothelix absolutelynothelix marked this pull request as draft June 17, 2023 14:16
@absolutelynothelix absolutelynothelix force-pushed the dependencies-cleanup branch 2 times, most recently from 2a1b74e to c3be2a5 Compare May 20, 2024 19:10
@absolutelynothelix absolutelynothelix force-pushed the dependencies-cleanup branch 2 times, most recently from f5d0796 to 62666b1 Compare May 30, 2024 11:17
@absolutelynothelix absolutelynothelix force-pushed the dependencies-cleanup branch 2 times, most recently from 54fc212 to 16617af Compare October 18, 2024 21:37
@absolutelynothelix absolutelynothelix changed the title core: dependencies cleanup core: separate work with the x extensions Oct 18, 2024
@absolutelynothelix absolutelynothelix marked this pull request as ready for review October 18, 2024 22:46
Copy link
Collaborator Author

@absolutelynothelix absolutelynothelix left a comment

Choose a reason for hiding this comment

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

@yshui, let's resolve a few concerns before merging this.

src/x.h Outdated Show resolved Hide resolved
src/utils/process.c Outdated Show resolved Hide resolved
@absolutelynothelix absolutelynothelix force-pushed the dependencies-cleanup branch 2 times, most recently from 5fd15d2 to f5302ee Compare November 9, 2024 14:36
@yshui yshui merged commit af3cf50 into yshui:next Nov 11, 2024
15 checks passed
@absolutelynothelix absolutelynothelix deleted the dependencies-cleanup branch November 11, 2024 18:32
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