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

Introduce geometry primitives #1862

Merged
merged 25 commits into from
May 7, 2024
Merged

Introduce geometry primitives #1862

merged 25 commits into from
May 7, 2024

Conversation

Caellian
Copy link
Collaborator

@Caellian Caellian commented Apr 25, 2024

Changes

This PR introduces point and rect primitives and replaces previous value pairs with them. It also replaces output-wayland.cc rectangle as it's effectively doing the same.

This aims to make future changes more consistent and to reduce number of global variables by grouping them. As a side bonus, it allows us to optimize some common operations a little by relying on Vc to make the compiler more aware of available opportunities to parallelize computation. Just grouping things results in significant reduction in number of generated move instructions:

image

Details

  • Added vec and rect structures.
  • Added Vc dependency for better SIMD optimization of common vector operations.
    • This was previously done through Eigen, but I removed Eigen as 90% of its functionality wouldn't ever be used by Conky so it only added to maintenance burdain.

These are based on Eigen and will basically get compiled away into ideal
representations for each platform conky is compiled for. This means that
many operations can be SIMD optimized with very little effort.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Copy link

netlify bot commented Apr 25, 2024

Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit c22dba9
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/6637e1c936cd2c000874ff76

@github-actions github-actions bot added sources PR modifies project sources dependencies Issue or PR that affects dependencies display: x11 Issue related to X11 backend lua Issue or PR related to Lua code mouse events Issue or PR related to mouse events labels Apr 25, 2024
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@github-actions github-actions bot added the gh-actions Issue or PR that suggest changing GitHub actions label Apr 25, 2024
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@github-actions github-actions bot added the display: wayland Issue related to Wayland backend label Apr 25, 2024
@Caellian
Copy link
Collaborator Author

Caellian commented Apr 25, 2024

@brndnmtthws Is it okay to add Eigen as dependency? I can do without it, but it might optimize some instructions. Alternatively, I could make it optional and apply SIMD only if present.

EDIT: We do surprisingly many vector computations while drawing components. I don't use majority of Eigen though. It might be better to simply add a way to provide optimized assembly for certain architectures and data types. 🤷‍♂️ And then someone can easily add optimizations later if they want to.

@Caellian Caellian force-pushed the fix/screen-geometry branch 2 times, most recently from 672fa4e to e3f8d3f Compare April 25, 2024 18:16
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
This should help avoid bad pointer reinterpretation.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian linked an issue Apr 25, 2024 that may be closed by this pull request
Also fixed bad previous substitutions. Will go through all of them at
the end just in case, though this time I replaced them one component at
a time.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian linked an issue Apr 25, 2024 that may be closed by this pull request
Copy link
Collaborator Author

@Caellian Caellian left a comment

Choose a reason for hiding this comment

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

Last commit is expected be functionally equivalent to original. Besides noted exceptions.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Added intermediate member access structures which map direct
member access and mutation into indirect one as provided by eigen an
most other representations. This should _almost_ always just work, but
in some cases template specialization consuming these values might fail
in which case dereferencing them should work.

Sadly there's no way of providing `T&` access to values because actual
data layout and representation can be wildly different between different
architectures. Even if eigen wasn't being used, if we want to make use
of SIMD we have to provide access through separate getters and setters.

All this abstraction should be optimized away by the compiler, so it's
just adding API ergonomics.

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian added the priority: low A low priority issue or PR label Apr 29, 2024
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@github-actions github-actions bot removed 3rdparty Issue or PR that suggests changes to 3rd party dependencies dependencies Issue or PR that affects dependencies build system Issues and PRs related to build system (CMake) and process labels Apr 30, 2024
@brndnmtthws brndnmtthws added the feature New feature PR or issue label May 1, 2024
@github-actions github-actions bot added 3rdparty Issue or PR that suggests changes to 3rd party dependencies build system Issues and PRs related to build system (CMake) and process and removed tests Issue or PR related to project tests labels May 5, 2024
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
@Caellian Caellian marked this pull request as ready for review May 5, 2024 19:45
@Caellian Caellian removed the priority: low A low priority issue or PR label May 5, 2024
Copy link
Owner

@brndnmtthws brndnmtthws left a comment

Choose a reason for hiding this comment

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

👍 , seems like Vc is a better choice than Eigen for our needs.

This is a pretty big change, so it's probably going to have some unintended side effects. There are a few things I'd refactor, but probably better to do it in a separate PR later.

@Caellian
Copy link
Collaborator Author

Caellian commented May 7, 2024

There are a few things I'd refactor, but probably better to do it in a separate PR later.

Same here 😆, I just tried adding semantics to rectangle via tempates (absolute vs. sized) and realized there's a lot that could be improved. C++ doesn't have traits as Rust does so it's somewhat tricky to get things like swizzling to layer properly and still allow the compiler to optimize code.

I'll probably address those issues at some point, but it's better to have something and I don't want to keep this PR waiting for too long as it will be painful to update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rdparty Issue or PR that suggests changes to 3rd party dependencies build system Issues and PRs related to build system (CMake) and process display: wayland Issue related to Wayland backend display: x11 Issue related to X11 backend feature New feature PR or issue lua Issue or PR related to Lua code mouse events Issue or PR related to mouse events sources PR modifies project sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants