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

Improvements to bounding boxes and rasterized layers #287

Open
JordanMagnuson opened this issue Feb 8, 2022 · 16 comments
Open

Improvements to bounding boxes and rasterized layers #287

JordanMagnuson opened this issue Feb 8, 2022 · 16 comments

Comments

@JordanMagnuson
Copy link

Layer bounding box shows most of the time when other tools are selected--it should only show when the move tool is selected for the current layer.

image

Video demo: https://www.loom.com/share/3a2da6ef83b9432b957e3ecfb3a6d468

@viliusle
Copy link
Owner

viliusle commented Feb 8, 2022

Maybe, but in your case how we should show that some layer is selected? We need more discussion here.

Other people opinion is welcome.

@Giwayume
Copy link
Contributor

Giwayume commented Feb 8, 2022

This is kind of a weird case, in many programs you never want a layer to be less than the size of the image itself so you can draw anywhere without hassle. When all layers are the same size as the image, a bounding box doesn't help you tell which layer is selected.

GIMP always shows a bounding box around the current layer no matter the current selected tool. Though it is a little more subtle. The dashed lines give it more of an appearance that it is not a part of the image itself. This is helpful for tools like the paintbrush so you know why you can't draw outside of these bounds.

image

Photoshop... I don't think it allows individual (raster) layers to be anything other than the resolution of the image itself. So in this sense, there's no reason for it to show a layer bounding box because layers aren't movable or resizable objects. You can move parts of an image using a selection and the transform tool, but this is temporary. You either apply the transformation immediately or cancel it before you can do anything else... when applied the pixels are written to the layer.

Minipaint Creates a new layer whenever you draw, which is unique compared to most other programs that need to draw on an existing layer.

Some clever art programs use animations to show you what's in the selected layer. For example, the second you select the layer all of the contents of the layer will zoom in and back out very quickly to indicate which part of the image was selected without having to draw anything extra on top of it.

I don't think you need to show the current selected layer bounding box when tools that create their own layers such as brush/pencil/shape/gradient are selected. It does help give context when clone/blur/bulge/pinch tools are selected, because it might be confusing why they aren't operating outside of certain bounds otherwise.

@JordanMagnuson
Copy link
Author

Hm... yeah, coming from Photoshop, it's strange to see the bounding box all the time, because in Photoshop it is basically never shown unless the transform tool is selected.

In terms of knowing what layer you have selected (for applying effects, etc.), you are expected to generallys simply rely on the layers panel--which works fine, and would generally work fine in miniPaint as well (graphic artists are used to this):

image

There are two issues with showing the bounding box all the time:

  1. The main, consistent problem: it is hard to get a "clean" view of your overall composition, becasue there is always a bounding box in the way--which is particularly annoying when coming from a program like Photoshop which hardly ever shows bounding boxes.

  2. In miniPaint in particular, the bounding box looks like it could be a stroke effect that is actually part of the image, which leads to additional confusion (this is particularly true when the box is shown without the transform handles).

Of course @Giwayume brings up a good point about the the size of the current layer being limited in miniPaint, so sometimes it is useful to know where the layer ends (e.g. when using the clone tool, etc.)

As usual, I would venture to say that following Photoshop's standards when possible is ideal, since, well, Photoshop tends to set the standards and expectations for these things.

Ideal solution (in my opinion):
Assuming it could be achieved performantly, make the drawable area of all layers the full size of the canvas, as Photoshop does, so that you can always clone/blur/bulge/etc within the full canvas area. Couldn't this be effectively achived by simply filling the layer out to the full canvas size after any move/transform operation?

The bounding box would then never need to be shown, except on move/transform (as in Photoshop).

Importantly, though, the bounding box in this case would need to be changed to bound the current filled area of the layer in question (as in Photoshop)--otherwise it would be meaningless in that it would always fill out to the full canvas size.

Less ideal solution:
As @Giwayume suggests, just show the bounding box for tools that draw onto the active layer, where knowning the bounds of the layer is useful, such as clone/blur/bulge/ping etc.

The question of what the bounding box defines is actually an important related question that I will link to shortly in another issue. There are several issues that arise in miniPaint currently because the bounding box for a given layer is taken to be the full size of the layer at all times, rather than the filled area within the layer (as in Photoshop).

@Giwayume
Copy link
Contributor

Giwayume commented Feb 9, 2022

I think the main disadvantage of Photoshop's approach is you lose essential information once you've pasted an image on a layer - you have to go back and manually select the image bounds again with the selection tool if you ever want to rotate it again with respects to the original pasted image's center (unless image bounds is exactly the canvas bounds, Ctrl + A).

I don't agree that Photoshop is the golden standard, it got a lot of things right but it's also pretty terrible in a lot of aspects, it was just first so a lot of people copy it.

I believe MiniPaint is intended to work more like a mockup editor similar to Adobe XD, where layer/shape size is important to keep.

@Giwayume
Copy link
Contributor

Giwayume commented Feb 9, 2022

Photoshop won't allow you to leave a layer in a state like this so you can come back and adjust it later. You are forced to apply the transformation before you can do anything else. It is by no means a non-destructive editor.

image

When you apply the transformation and create a new raster image, you are losing information about the original image. If you need to change it later, a 2nd transformation on top of an already applied transformation is going to look worse than if you got it right the first time. An ideal image editor would apply transformations at the time the image is exported (or when the user asks for it).

@JordanMagnuson
Copy link
Author

JordanMagnuson commented Feb 9, 2022

@Giwayume My point about Photoshop is not to argue that it is the "gold standard" in terms of being the perfect/ideal image editor. (That may or may not be true, but is likely not true for a variety of reasons.) My point is that it sets the standards, meaning that it sets the expectations for the ways a raster image editor works, for just about anyone who works with raster images.

Photoshop won't allow you to leave a layer in a state like this so you can come back and adjust it later. You are forced to apply the transformation before you can do anything else. It is by no means a non-destructive editor.

This is no longer true. Recent versions of Photoshop actually work very much like miniPaint: images are brought in as "smart layers" which retain the original image bounding box (regardless of filled pixels) and can be transformed without loss of information until the layer is rasterized.

image

Actually, messing around more with miniPaint today I realized that it works a lot more like Photoshop than I previously thought. There is not much difference in how it handles the bounding box and transforming of non-rasterized layers ("smart layers" in Photoshop). Where there is a difference is in how it handles rasterized layers. (Part of the confusion here for me stemmed from the fact that there is not a clear indicator in minitPaint of whether a layer is rasterized or not, and miniPaint converts rasterized layers back to being non-raster after any transform).

When you rasterize a layer in miniPaint, the layer expands to fill the full canvas (meaning the effective draw area), much like Photoshop. There are two differences at this point:

  1. If you transform the newly rasterized layer in miniPaint, it goes back to being non-rasterized, and must be rasterized again manually. Photoshop automatically keeps rasterized layers rasterized for the duration. (Each of these approaches has its pros and cons, but you could get the benefits of both by simply having an option to "keep rasterized layers rasterized" or soomehting like that (which would automatically apply rasterize after any transform operation on a layer that was previously rasterized).

  2. Getting back to the bounding box issue: in miniPaint the bounding box of a rasterized layer extends to the full size of the Canvas, in order to match the fact that the layer itself extends to the full size of the canvas. In Photoshop, by contrast, bounding boxes on rasterized layers are "smart" in that they adjust to the filled pixel area of the layer (the "image size"), rather than extending the bounding box to the full canvas area. Since the original layer size has already been lost at this point, I dont' think there's any strong argument that can be made to claim that it's more useful to have the bounding box equal the full size of the canvas in this situation, rather than have it smartly bounded to the actual image size. This is particularly true for small images on large canvases, where it becomes somewhat absurd to have to zoom way out to the full canvas size in order to rotate a single small image on a rasterized layer.

My proposal at this point woud be to make three changes to miniPaint:

  1. As previously suggested, for non-rasterized layers, just show the bounding box for tools that draw onto the active layer, where knowning the bounds of the layer is useful, such as clone/blur/bulge/ping etc.

  2. Have a settings option to "automatically rasterize new layers" (this would just rasterize everything all the time), as well as an option to "always keep rasterized layers rasterized" (this would keep layers rasterized if they had previously been manually rasterized). Both options off by default.

  3. For rasterized layers, employ a "smart" bounding box as Photoshop does, which extends to the trimmed pixel dimensions of the layer (i.e. the "image size"), rather than having the bounding box extend to the full canvas size, which is not very useful.

(For rasterized layers, miniPaint already has "correct" bounding box visibility--it only shows the layer bounding box when the move tool is selected.)

A possible 4th change would be to improve layer indicators in the layers panel in order to more clearly distinguish between rasterized and non-rasterized layers in miniPaint.

I'll make a video shortly which attempts to explain these suggestions with clear example reference to both miniPaint and Photoshop.

@JordanMagnuson
Copy link
Author

Okay, here's a showrt video showing rasterized layer behavior and bounding box differences in Photoshop vs. miniPaint: https://www.loom.com/share/2f7c9f8a42594b9f8c0409e30cf3430d

@JordanMagnuson JordanMagnuson changed the title Layer bounding box should only be visible when move tool is selected Improvements to bounding boxes and rasterized layers Feb 9, 2022
@Giwayume
Copy link
Contributor

Giwayume commented Feb 9, 2022

Hmmm I've been testing in my old CS6 install which for some reason crashes every time I do a 4-5 things so it's difficult to test. It's definitely not using smart objects by default when you add images. The initial bounding box acts like the transform tool in that it lets you resize but it requires you to place & rasterize it to a layer before you can do anything else, like select a different layer. Maybe the new "pay until you die" version doesn't do this.

image

Convert to smart object always crashing for me, so oh well.

I think the terminology is getting a little muddied here, when you open an image in Minipaint, it is always a raster layer. Even if you rotate and scale the layer, it's just a raster layer with a transform applied to it. So the pixels are drawn in a different coordinate system from the image canvas, but the layer itself is still an image that can be edited per-pixel basis.

What's nice about Minipaint layers compared to Photoshop smart objects, from what I see, you can't edit smart objects directly on canvas. You have to go through a process to open it as its own image then import it back. Minipaint tools can edit scaled & translated raster layers directly. Though, it does have lots of problems with rotation in general, including disabling editing when a layer is rotated. All of that needs work.

When you rotate/scale a raster layer in Minipaint, then use "Layer -> Convert to Raster", what it's doing is creating a new raster layer that is the size of the canvas, 1:1 scale and no rotation, then drawing the previously transformed raster layer on to that. In order to duplicate the Photoshop behavior you desire, it would need to still do the same thing except run an auto-crop on the layer afterwards to trim the transparent pixels.

@viliusle

A hint on rotation, Minipaint should drop the "x", "y", and "rotate" variables, and instead use a single DOMMatrix object. This way, calculating for a transformed point (rotated, scaled, translated) is all handed to you for free by the browser.

const layerTransform = new DOMMatrix();
const transformedPoint = new DOMPoint(mouseX, mouseY).matrixTransform(layerTransform.inverse());

You can use "transformedPoint" in this sense to compare directly to the layer as if it were a non-rotated, non-scaled, non-translated image. Converting all of this is probably going to be a large refactoring task, though.

@JordanMagnuson
Copy link
Author

JordanMagnuson commented Feb 10, 2022

I think the terminology is getting a little muddied here...

Yes, I agree. The problem is that both Photoshop and miniPaint have an operation called "rasterize", even though in both cases the layers being "rasterized" generally contain raster images.

When I draw a distinction between "rasterized" and "not rasterized" layers, I mean that in both editors there are two different states a layer can be in. In Photoshop, you have a smart object, or a rasterized layer, and many operations can only be done on "rasterized" layers. In miniPaint, similarly, many operations (e.g. erase, magic wand, paint bucket, etc.) cannot be done on layers which are considered "rotated"--and "rasterizing" the layer allows those operations.

image

@JordanMagnuson
Copy link
Author

In order to duplicate the Photoshop behavior you desire, it would need to still do the same thing except run an auto-crop on the layer afterwards to trim the transparent pixels.

Would this allow you to still "draw" outside of the cropped image bounds, though? (meaning clone, blur, fill, etc.)

@Giwayume
Copy link
Contributor

Giwayume commented Feb 10, 2022

Would this allow you to still "draw" outside of the cropped image bounds, though? (meaning clone, blur, fill, etc.)

No, but it isn't really that hard to make it so those tools don't have to respect the original layer bounds, though. They're already creating a new draft canvas and drawing on that instead of the original image.

https://github.com/viliusle/miniPaint/blob/master/src/js/tools/clone.js#L226

similarly, many operations (e.g. erase, magic wand, paint bucket, etc.) cannot be done on layers which are considered "rotated"--and "rasterizing" the layer allows those operations.

Same thing if you set the rotation back to 0. It not working with rotation is really just a lack of knowledge in implementation.

@viliusle
Copy link
Owner

Sorry for now reply, but I do not have time right now. Once we finalize everything, I believe implementation will be simple. For example even now you can rasterize object and use trim without border option to get minimized area.

p.s. @Giwayume I will check your hint about DOMMatrix.

@Giwayume
Copy link
Contributor

Giwayume commented Feb 10, 2022

If you need to extract the rotation, scale, translation individually from a DOMMatrix, you can use this function.

function decomposeMatrix(matrix) {
    let a = matrix.a, b = matrix.b, c = matrix.c, d = matrix.d, e = matrix.e, f = matrix.f;
    let scaleX, scaleY, skewX;
    if (scaleX = Math.sqrt(a * a + b * b)) a /= scaleX, b /= scaleX;
    if (skewX = a * c + b * d) c -= a * skewX, d -= b * skewX;
    if (scaleY = Math.sqrt(c * c + d * d)) c /= scaleY, d /= scaleY, skewX /= scaleY;
    if (a * d < b * c) a = -a, b = -b, skewX = -skewX, scaleX = -scaleX;
    return {
        translateX: e,
        translateY: f,
        rotation: Math.atan2(b, a),
        skewX: Math.atan(skewX),
        scaleX: scaleX,
        scaleY: scaleY
    };       
}

@JordanMagnuson
Copy link
Author

Same thing if you set the rotation back to 0. It not working with rotation is really just a lack of knowledge in implementation.

I was wondering about this. So you think we can get all tools/effects working on rotated layers?

My updated thoughts for changes to miniPaint based on our recent discussion:

  1. Option/setting to "automatically rasterize rotated layers." (The reason for this being that so many tools/effects cannot currently be performed on rotated layers.)
  2. Option/setting to "automatically trim/crop rasterized layers." (Instead of staying at full canvas size.)
  3. Option/setting to "allow drawing outside of layer boundaries" (for blur, bulge, pinch, etc.) -> layer would then automatically be expanded after operation.

But if it's possible to get all tools/effects working on rotated layers, then option 1 is unecessary, in which case:

  1. Implement all tools/effects to operate on rotated layers.
  2. Option/setting to "automatically adjust layer size after image manipulation." -> This would expand the layer if necessary (e.g. if drawing outside layer boundaries), and then trim/crop the layer down to the current pixel dimensions (e.g. after erase/delete).

Thoughts?

@Giwayume
Copy link
Contributor

The latter is the way to go, honestly. All of the brush-type tools are perfect circles right now anyways so it's not like that's going to add any complication to the implementation with rotation.

@JordanMagnuson
Copy link
Author

Okay, I've opened two separate issues to tackle these changes:

#299

#300

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

No branches or pull requests

3 participants