-
-
Notifications
You must be signed in to change notification settings - Fork 383
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
Reference Images #771
Reference Images #771
Conversation
This looks great! Custom cel render logic is intended to be placed in the
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 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? |
This would definitely be unusable for reference purposes as this is sort of implying that it'll show up in the export
You can in fact display the reference to the side of the canvas, yes |
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. |
Yeah, that definitely... would not have been fit for the purpose |
Overall this looks good and it seems to be working well. I have some comments about the UI before merging though.
|
While handling these: While there are no conflicts, should I rebase anyway? |
Rebasing is not necessary since there are no conflicts. |
e28fbba fixes all three of these:
Done.
Autowrap is now enabled on all the references labels.
There wasn't, I've fixed it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good to merge now. Thank you very much for the contribution!
Thank you, also. |
...So I mentioned in the TODO in the PR description:
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? |
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). |
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.TODO:
Rationale of implementation decisions:
Layers were not used as the state of the code makes this difficult.
Neither
BaseCel
orBaseLayer
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.