markusfisch / ShaderEditor

Android app to create GLSL shaders and use them as live wallpaper
https://play.google.com/store/apps/details?id=de.markusfisch.android.shadereditor
MIT License
924 stars 137 forks source link

feat: Highlighting based on c-based lexer #145

Closed AntonPieper closed 1 year ago

AntonPieper commented 1 year ago

I also tried using treesitter (see treesitter-highlight), however it was too slow for massive (say 4k LOC) files. Weirdly this lexer is super fast on my M1 MacBook but not on my android phone.

In the future a parser could be used asynchronously to improve highlighting even further (i.e. local variables).

Also this switches the build system and support library to the newer AndroidX and uses libs.versions.toml for easier version managment.

Closes #140

markusfisch commented 1 year ago

Thanks a lot for implementing this! πŸ‘ Impressive work!

Now, I will have to try and profile this before I can merge it, of course. Also, there's a lot going on which isn't related to the C-based highlighter (like the migration to AndroidX or version catalogs). Since I would squash the PR, this would pack a lot of different unrelated changes into one big commit - what would make it very hard to track down individual changes or actually work with the git history. So, frankly, I would prefer a PR for every feature/topic 😬

Also I noticed that you changed the indent of ShaderRender.java to two spaces, which is something I just can't live with πŸ˜‰πŸ€·β€β™‚οΈ After all, source code is meant to be read by people, and in my experience it really pays to have a common standard so that a project doesn't get completely muddled. This project is 10 years old now πŸ˜‰

AntonPieper commented 1 year ago

Oh, I didn't notice that I formatted with two spaces😬. I will split this pull request up into multiple. Also I probably will work a bit more on the performance, it seems like the building of the spanned text takes the most time.

I will open pull request for the following:

If it works out, I will also add:

AntonPieper commented 1 year ago

Closing this in favor of the individual pull requests.