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

Painting improvements #1267

Merged
merged 5 commits into from Dec 1, 2019
Merged

Conversation

scribblemaniac
Copy link
Member

@scribblemaniac scribblemaniac commented Sep 21, 2019

This is a change I've wanted to do for a while now. I was playing with the feather rendering when I thought to myself, "hey I can probably do this now", and it turns out I could! When using the drawing tools, they don't always look the same as when you finish drawing. The eraser acts like a white brush and all brushes draw over top of everything, even if you are drawing on a lower layer. These were necessary tradeoffs related to how we cached the frame while drawing. However, now I've improved these behaviors and have adapted the caches to work as well as possible (which should be fairly close to the original performance). Take a look:

Live eraser preview revealing the layer below:
Painter

Drawing a stroke on "cur layer" with "pre vec" and "pre bit" below and "post vec" and "post bit" above the active layer:
Ordering

These changes also open up the opportunity to use different blend modes with our brushes. For instance, we could easily add alpha locking and burn/dodge tools with this. See #1158

Will conflict with #1160. The preview while drawing with a tablet will not work, but that will be fixed in #1269 which will be merged before this anyway.

In particular this changes two big things:
1. The render order. Now the buffer image is rendered directly
above the active layer, rather above all layers.
2. The eraser tool no longer paints white, it erases the current
layer while drawing.
@scribblemaniac scribblemaniac added Enhancement Canvas 🔷 Major PR (two reviewers when possible) labels Sep 21, 2019
@Jose-Moreno
Copy link
Member

Jose-Moreno commented Sep 22, 2019

@scribblemaniac First off, thank you for taking the time to work on this! Although I'm not sure I understand the first GIF, does this mean we can now paint with alpha as well as erase? if we can use fully transparent alpha values we can for example, use the fill tool to color a shape and replace the currently solid color instead of painting over it. Or is that a totally different thing?

If it is a different thing, could that idea be considered a blending mode then? Don't know if you're familiar with Blender's Add Alpha / Erase Alpha blending modes for example, where adding alpha makes the color fully transparent (but somehow it keeps the color instead of erase it) and erasing alpha makes the color fully opaque (modified by pressure of course) so the color is revealed.

Since we're using PNG under the hood and PNG's use straight alpha (that means the color comes RGBA'd instead of premultiplied through transparency masks), we should be able to use that where the color itself is transparent, this of course would count as part of the surface bounding box.

Also is the second GIF what I think it is? could we finally get a way to paint behind the lines? if so that would honestly be a most desired feature!!! and it would also solve a LOT of issues for users have when coloring in the same layer (i.e white halos and incomplete fills due to threshold with anti-aliased strokes)

If we couple this with the upcoming coloring features, Pencil2D users will seriously have no need to use other programs the coloring workflow.

Basically I think this GIF is appropriate for your PR's 😁

Daniel_Bryan_YES

@scribblemaniac
Copy link
Member Author

scribblemaniac commented Sep 22, 2019

Although I'm not sure I understand the first GIF, does this mean we can now paint with alpha as well as erase? if we can use fully transparent alpha values we can for example, use the fill tool to color a shape and replace the currently solid color instead of painting over it. Or is that a totally different thing?

That's a different thing by the sounds of it, although maybe you could think of the eraser tool as working that way if you wanted. The first gif is of the eraser tool. There's a layer of red, and then a layer with the text below. The red is being erased in real-time, rather than after you finish your stroke. Painting with alpha will add the color on top of the canvas like before. A sort of "clear mode" for the bucket fill was possible before this and is possible after too, it just needs to use a different composition mode.

could we finally get a way to paint behind the lines?

Again, not really. There are no changes to the tools in this, only in how the are rendered while in use. So you can paint behind lines, but you will need to be on a lower layer, just like right now. The difference is that now you can actually see what you're doing 😋

The Add/Erase Alpha and drawing behind lines on the same layer should both be possible with different blending modes for the brushes.

I appreciate the enthusiasm 😁

* remove unnecessary qscopedpointer allocations for qpainter objects and use normal object instance with references.
* Hide logic from scribblearea
Copy link
Member

@MrStevns MrStevns left a comment

Overall I think this is very nice, It's so welcoming to see some changes to the painting logic, so that we can see layer changes in real-time.

I have a few technical issues with the current structure vs how it was that I'd like to see changed before we merge though. Currently I think there are too many points of entry in CanvasPainter and scribbleArea knows too much about CanvasPainter.

My thought process on the painter class pattern is that that we keep such classes very restricted, with little context of the outside world, ScribbleArea and other classes should only tell a painter when to paint and the context it needs to paint, to hide the rendering pipeline as much as possible.

I've made a PR on your fork that should follow this idea more closely.

core_lib/src/canvaspainter.h Outdated Show resolved Hide resolved
@MrStevns MrStevns added this to the 0.6.5 milestone Nov 10, 2019
@MrStevns
Copy link
Member

MrStevns commented Nov 24, 2019

It would be nice to have this merged, so we can begin looking at #1160

@scribblemaniac would it be possible for you to fix the remaining conflicts soon?

@scribblemaniac
Copy link
Member Author

scribblemaniac commented Nov 24, 2019

@candyface Yes I'll try to fix the conflicts soon. Just keep bugging me if I don't 😉

@scribblemaniac
Copy link
Member Author

scribblemaniac commented Dec 1, 2019

@candyface Alright, ready to be merged now. Sorry for the delay.

@MrStevns MrStevns merged commit 91951c5 into pencil2d:master Dec 1, 2019
scribblemaniac added a commit to scribblemaniac/pencil that referenced this pull request Mar 29, 2020
The polyline tool was not correctly displaying while drawing. This
was probably broken by the changes to the frane caching in pencil2d#1267.

I also did a little bit of refactoring to implement this more
cleanly. Tools now report if they are modifying the canvas with
isActive, rather than ScribbleArea trying to figure that out.
I also changed the right click action to set the temporary tool to
the hand tool rather than forwarding move event to the hand tool.
This is the same way that the middle mouse button works and is
slightly better because it will show the hand tool cursor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Canvas Enhancement 🔷 Major PR (two reviewers when possible)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants