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

Enhanced relative layer visibility #1160

Merged
merged 32 commits into from Mar 4, 2020

Conversation

MrStevns
Copy link
Member

@MrStevns MrStevns commented Jan 19, 2019

reworked relativity

I have also implemented the ability to select the startup value for the global dot visibility mode.
image
image

fullfills and closes #1112

This PR adds:

  • Brings back relative layer visibility (see above)
    ~~- This brings back relative layer transparency with slight enhancements.
  • Left click to increase dot index, right click to decrease dot index
  • Ability to control relative visibility through preferences
  • Customizable layer visibility shortcuts
    • default shortcuts are alt+1, alt+2, alt+3
  • Semi-Opaque (Transparent) Layers: Active layer is opaque. Passive Layers are transparent.
  • Fully Opaque Layers: Active / Passive Layers are opaque by default
  • Solo Layers: Active layer is opaque. Passive Layers are hidden from the user.
  • Shortcut Key(s) Combination: Cycle Forwards / Cycle Backwards
  • Make fully opaque layer state default instead of semi-opaque / transparent state

Given the state of the timeline, I've left these out. I personally think that relative layer visibility should still be default, at least until it becomes apparent enough what the dots are used for (ie. to be changed via the timeline rewrite.)

  • Move "Solo" State to Layer "visibility" manager

Not implemented exactly as requested:

  • Widget Key Modifier: Inverse Cycling Order
    You can cycle backwards using right-click now,

MrStevns added 4 commits Jan 19, 2019
This brings back relative layer transparency with a slight enhancement or similarity to the real world.
The top layer is at the bottom of the visibility stack, meaning that once the top is selected, other layers aren't shown. As soon as you move down the stack (or add more papers to the light board) then the drawings behind will become more and more opaque.
Adds:
+ Shortcuts to change overall layer visibility
+ Ability to toggle visibility backwards using right-click
@Jose-Moreno
Copy link
Member

Jose-Moreno commented Jan 21, 2019

@candyface YES!!!!!!! Thank you so much! I know many people will love to have this back, myself included! 😄

@Jose-Moreno
Copy link
Member

Jose-Moreno commented Feb 5, 2019

@candyface Alright I've read through all your notes. I just have one question. Will the visibility state be saved in a working pencil2d file? or will it be saved in the pencil2d preferences?

I mean It's ok that transparent is default for a new user, but if the user presses alt +3 to make it fully opaque, will this change revert the next time they either open the program or open a new file?

@MrStevns
Copy link
Member Author

MrStevns commented Feb 10, 2019

@Jose-Moreno The visibility state is not saved atm. so when you reopen a project it'll start as relative like it always has. I'll look into seeing if I can add that to the project file.

edit: correction, it is saved but it is being set to 1 (relative) when loading the xml. I will fix that.

@MrStevns
Copy link
Member Author

MrStevns commented Feb 10, 2019

@Jose-Moreno The visibility state is now being saved and loaded as requested.

Copy link
Member

@scribblemaniac scribblemaniac left a comment

Personally I don't think it's good idea to have this feature enabled by default. I'm not debating the usefulness of this feature to some people, but I think the majority of our users, particular the ones new to animation, will see this as a bug. Let's not forget that the reason this was removed in the first place was because of the numerous complaints about parts of their drawing becoming translucent. The 'improvements' in this PR will only make things worse. People will create a layer for their background and then a character layer on top of it and think "wtf where is my background? this program sukz" or something along those lines.

@MrStevns
Copy link
Member Author

MrStevns commented Mar 3, 2019

reworked relativity

I've reworked the algorithm now so it goes both ways. So now it's "true" relative opacity based on the current layer in each direction as opposed to before where it only went downwards and the top layer would be the bottom of the stack and the only visible one.

Additionally I have also implemented the ability to select the startup value for the global dot visibility mode.
image
image

@scribblemaniac That's true I almost forgot about that.
Showing all layers fully opaque is now the default option.

@Jose-Moreno A correction from my last comment, the visibility state that I was speaking about was simply whether a layer was visible or not, since that wasn't handled either. The global layer visibility state was not loaded or saved prior to my last commit but now it's possible to set the startup value through the preference dialog. It is not project based currently.

@MrStevns MrStevns requested a review from scribblemaniac Mar 3, 2019
@MrStevns MrStevns requested a review from chchwy Apr 13, 2019
@MrStevns MrStevns force-pushed the feature/layer-transparency branch from 2c5c49d to bcae158 Compare Sep 11, 2019
@MrStevns MrStevns force-pushed the feature/layer-transparency branch from bcae158 to 787d9a4 Compare Sep 11, 2019
@MrStevns
Copy link
Member Author

MrStevns commented Sep 11, 2019

PR now up to date and everything still works.

@scribblemaniac scribblemaniac added this to the 0.6.5 milestone Sep 13, 2019
@scribblemaniac scribblemaniac added the 🔷 Major PR (two reviewers when possible) label Sep 13, 2019
@MrStevns
Copy link
Member Author

MrStevns commented Dec 8, 2019

Finally got around updating the code after the other painting changes, can be reviewed now.

MrStevns and others added 4 commits Dec 9, 2019
Because the parent implementation of mouseDoubleClickEvent was
never being called, the second mouse event never fires, causing
some mouse presses to not change the visibility. This wasn't
a big issue with most of the other timeline functions because
clicking twice in the same place usually has the same effect as
clicking once or a double click action.
Copy link
Member

@scribblemaniac scribblemaniac left a comment

It's almost there, but there are still some changes to be made. Many minor refactoring things, a couple functional things, and a couple points to discuss further.

In addition to what is mention in my review comments, I have one further issue to note that I did not have a good place to put: the dot color is not updated to the correct value when Pencil2D is started. You probably need some extra code in timelinecells

app/src/mainwindow2.cpp Outdated Show resolved Hide resolved
app/ui/timelinepage.ui Outdated Show resolved Hide resolved
app/src/preferencesdialog.cpp Outdated Show resolved Hide resolved
core_lib/src/interface/scribblearea.cpp Outdated Show resolved Hide resolved
app/src/preferencesdialog.cpp Outdated Show resolved Hide resolved
core_lib/src/managers/preferencemanager.cpp Outdated Show resolved Hide resolved
core_lib/src/managers/preferencemanager.cpp Outdated Show resolved Hide resolved
core_lib/src/structure/layer.cpp Outdated Show resolved Hide resolved
core_lib/src/structure/layercamera.cpp Show resolved Hide resolved
core_lib/src/structure/layer.cpp Show resolved Hide resolved
MrStevns added 3 commits Jan 13, 2020
Add default layer values if none is found

Fix comparison of floating pointer warning

Fix optionid being float where it should be int

Use layerVisibility enum instead of int consistently

Always render layers on camera layer

Simplify condition

Fix warning

Fix cosmetics

Add layer opacity threshold as parameter to CanvasPainterOptions

Fix spinbox not going beyond 52...

Move layer and onion skin into own section

Move layer visibility to View instead of Layer

Fix UI bugs

Fix warning and remove debug statement

LayerVisibility should be 2 by default

Refactor some cosmetics

Remove redundant check
@MrStevns MrStevns requested a review from scribblemaniac Jan 15, 2020
Copy link
Member

@scribblemaniac scribblemaniac left a comment

Next batch of comments ready to go. Additionally, the keyboard shortcuts do not appear to be working anymore.

app/src/preferencesdialog.cpp Show resolved Hide resolved
app/ui/timelinepage.ui Show resolved Hide resolved
core_lib/src/canvaspainter.h Outdated Show resolved Hide resolved
core_lib/src/interface/scribblearea.cpp Outdated Show resolved Hide resolved
core_lib/src/interface/timelinecells.cpp Outdated Show resolved Hide resolved
core_lib/src/interface/scribblearea.cpp Outdated Show resolved Hide resolved
core_lib/src/structure/layer.cpp Outdated Show resolved Hide resolved
core_lib/src/interface/scribblearea.cpp Show resolved Hide resolved
@MrStevns MrStevns requested a review from scribblemaniac Jan 17, 2020
@scribblemaniac scribblemaniac self-assigned this Mar 4, 2020
Copy link
Member

@scribblemaniac scribblemaniac left a comment

Aside from a couple very small things which I'll just handle myself, I think this is finally ready. Merging now, thanks for your work and patience @candyface.

@scribblemaniac scribblemaniac merged commit 0a2f8f2 into pencil2d:master Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement 🔷 Major PR (two reviewers when possible) Layer Management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bring back "relative transparency" for layers a.ka restore the Light Table feature
4 participants