Curves image adjustment node#1214
Conversation
Deploying with
|
| Latest commit: |
55dd406
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ae4c9a0b.graphite.pages.dev |
| Branch Preview URL: | https://curves-node.graphite.pages.dev |
✅ Deploy Preview ready!
|
|
@Keavon Dennis said, that you gonna have opinions about the curves widget I've implemented. In my implementation the backend and frontend communicate via a list of curve samples and the frontend composes the resulting svg. At the moment the backend works with cubic splines and the frontend works with bezier (a quick wikipedia look said me they are bijective), but probably it's the best to replace the backend (cubic) spline implementation with bezier-rs. Dennis also showed me https://github.com/GraphiteEditor/Graphite/tree/master/website/other/bezier-rs-demos. I guess I'll leave my frontend code as it is and only steal the neat design from there. |
|
Looks like this is taking shape and starting to work now! I'm noticing that it breaks (and throws a console error) after a little while of dragging the points around, at which point the laggy UI stops being laggy (since it's not doing anything anymore from then on). |
Could you please provide an error log? I can't reproduce that. |
* move spline code into seperate module
* add a `LuminanceMut`-trait * add `lerp` to `Channel`
the first event issued to the debouncer was unneccessarily delayed. Instead now the debouncer fires it instantaneously but blocks events that come in until a timeout was reached.
It should definitely just happen by using it for a bit, maybe try and be aggressive and drag it around a lot every frame. I'm using Chrome in case that makes any difference. I'm adding this downstream of the Downscale node for a 4000x3000 image. The only error it prints is this:
Note, that is not referring to hitting a Rust |
|
Huh, interesting. This looks like an out of memory error. My first intuition is that this happens, because too many non-completed events stack up in the event queue. But I'll have |
* sample -> manipulator group or manipulator * marker -> handle
also integrate the function into curves node
|
I've updated the branch to use the new master, I think it makes sense to merge this pretty soon (even if it is not completely done) to prevent further code rot |
|
This does acutally work surprisingly well, I can fix the tests and I think we might be good to merge then |
|
I built this branch and don't see the Curves node in the layer add menu, do you know what's up with that? |
|
@nat-rix do you know what would most likely be the culprit for the failing tests? |
The build link works, can you retry that? |
|
@Keavon It is unfortunate you had to revert the brightness contrast changes. It would be very good if we could reuse that code instead of duplicating the entire file + we lose the improvements to the code structure |
|
Yes, but that'll have to be a future contributor's task since the more important priority right now is just getting this merged in its current (plus slightly fixed up) state. |
3 participants
No description provided.