Reference Images

[20kdc]

Affects #565.

Usecase:

A friend of mine was looking for a pixel editor that was, well, not the one we've all heard of, and considers high-resolution reference images to be critical to her rotoscoping workflow.

Images:

Usage:

• New reference image is a new image open type.
• Reference images are not stored in the project file due to size reasons.
• Reference images may be scaled and positioned in project space in a newly added panel (currently appearing as a tab of Tools, which is unfortunate)

TODO:

• Figure out how to make it work with the UI layout
• Anything I forgot (it appears that Pixelorama code requires changing things in a lot of places, and it's possible I forgot something)

Rationale of implementation decisions:

Layers were not used as the state of the code makes this difficult.

Neither BaseCel or BaseLayer are capable of providing custom render logic (which seems odd - even if GroupCel did nothing as part of what appears to be a general 'flattening' of cels, it would still be pertinent to have the cels perform rendering by themselves given the clear intent of the design)

In addition, the assumption would be that reference layers probably do not want to be disposable cels (as a new blank frame would cause the reference image to disappear, which is not necessarily wanted)

Guides on the other hand have complete control of their rendering, and therefore I chose the design of guides as the basis of the feature.

Reference images are expected to be large and probably in a format such as PNG or JPEG. Therefore reference images are linked by absolute file path. This is up for potential change but doing so could be hazardous.

It's unclear how exactly changes are expected to propagate - I presently assume project_changed is the global "anything important was modified" function, and this appears to work reliably enough.

[mrtripie]

This looks great!
project_changed is called when changing between projects (ie: opening a new one or changing project tabs).

Custom cel render logic is intended to be placed in the update_texture method of classes extending BaseCel. image_texture should be updated there, and in Canvas.gd the image_texture of each cel in the current Frame are blended together.

PixelCel converts its image (used on the CPU) to image_texture (used on the GPU) there.

I'm working on a vector layers branch as another example: https://github.com/mrtripie/Pixelorama/blob/VectorLayers/src/Classes/VectorCel.gd (near the bottom)

Currently GroupCels don't flatten their children cels, but probably will some time in the future.

I agree that your current approach is better than having it as a layer though. You should be able to do things like display the reference off to the side of the canvas right?

[20kdc]

Custom cel render logic is intended to be placed in the update_texture method of classes extending BaseCel. image_texture should be updated there, and in Canvas.gd the image_texture of each cel in the current Frame are blended together.

This would definitely be unusable for reference purposes as this is sort of implying that it'll show up in the export

I agree that your current approach is better than having it as a layer though. You should be able to do things like display the reference off to the side of the canvas right?

You can in fact display the reference to the side of the canvas, yes

[mrtripie]

This would definitely be unusable for reference purposes as this is sort of implying that it'll show up in the export

The cels are also blended together when exporting, so reference layers could be excluded in export. (Canvas's blending is just for seeing the end result in Pixelorama, I think it's also blended together separately in the small preview panel usually in the top right)

If it used layers, it would be restricted to the project's resolution, and the boundary of the Canvas though.

[20kdc]

If it used layers, it would be restricted to the project's resolution, and the boundary of the Canvas though.

Yeah, that definitely... would not have been fit for the purpose

[OverloadedOrama]

Overall this looks good and it seems to be working well. I have some comments about the UI before merging though.

1. Instead of SpinBox nodes for the reference image properties (ReferenceImageButton.tscn), we should use the custom ValueSlider node instead. It's a node introduced in v0.11 and it can be found under src/UI/Nodes/ValueSlider.tscn. It essentially combines a spinbox and a slider, allowing for easier moving around/scaling the reference image, without necessarily having to put numeric values every time we want to make a change.

2. In the ReferencesPanel, the Label should have autowrap enabled. Without it, the panel cannot be resized to a size smaller than the width of the text. This is very likely to cause issues with smaller screens and localization, where the translated text may be even bigger than the English one.

3. In ReferenceImageButton.tscn, I noticed that the Opacity option allows for values smaller than 0 and bigger than 100. Is there any reason for this?

[20kdc]

While handling these: While there are no conflicts, should I rebase anyway?

[OverloadedOrama]

Rebasing is not necessary since there are no conflicts.

[20kdc]

e28fbba fixes all three of these:

1. Instead of SpinBox nodes for the reference image properties (ReferenceImageButton.tscn), we should use the custom ValueSlider node instead. It's a node introduced in v0.11 and it can be found under `src/UI/Nodes/ValueSlider.tscn`. It essentially combines a spinbox and a slider, allowing for easier moving around/scaling the reference image, without necessarily having to put numeric values every time we want to make a change.

Done.

2. In the ReferencesPanel, the Label should have autowrap enabled. Without it, the panel cannot be resized to a size smaller than the width of the text. This is very likely to cause issues with smaller screens and localization, where the translated text may be even bigger than the English one.

Autowrap is now enabled on all the references labels.

3. In ReferenceImageButton.tscn, I noticed that the Opacity option allows for values smaller than 0 and bigger than 100. Is there any reason for this?

There wasn't, I've fixed it.

[OverloadedOrama]

Should be good to merge now. Thank you very much for the contribution!

[20kdc]

Thank you, also.

[20kdc]

...So I mentioned in the TODO in the PR description:

• Figure out how to make it work with the UI layout

I have noticed that no further changes were made to the PR to facilitate doing this properly, so I'm assuming you're handling it on your end on master?

[OverloadedOrama]

From my testing, it seems to already work well with the UI layout system. There was a crash when dropping the References tab into the main canvas, but this is unrelated to this PR and it got fixed when updating the addon to version 1.1.1 (927e4f4).