-
Notifications
You must be signed in to change notification settings - Fork 293
Statusbar update with zoom and Timecode/Frames/NoText #1423
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
Conversation
|
@davidlamhauge Nice job David! Sorry for not commenting much on your PR's and contributions lately, I've been very busy 😖 On the matter of MM:SS:FF, I understand why you took out the hourly digits out, though I've seen programs that simply leave it with a single H digit as in H:MM:SS:FF when it's not sure to reach hourly long footage so e.g 10 seconds at 30FPS would be -> 0:00:10:30 Well I don't think it's a big deal anyway as long as people understand what is it they're looking at, and right now it's at least clear which system is being used 👍 On a more technical level related to usability, would it be possible to get rid of the little dropdown box and make the label itself behave as the selection menu instead? I think clicking on the timecode itself would be nice. If that's not possible, I suppose changing the icon would help to make it more obvious that you can change the timecode format, otherwise it looks as if you can change the zoom% instead (which would also be nice! I'll ask for that later 😆 ) Other than that I think it will be a welcome addition. Thanks for looking out for all of these issues! 😊 |
|
Thanks for the feedback, @Jose-Moreno ! About a digit for Hours, I'll wait and see what the other think. I don't think it's needed. The tool button in the statusbar is there, because I was afraid that we would enter a 'How to change camera size'-problem, if it wasn't a noticable button. I do see, that if you choose 'No text', it will look like the button is about the zoom, so maybe there should be three slashes '---', periods '...' or something else, to indicate that there in fact is a label. |
|
I have not had a chance to test or look at the code yet, but I'm liking the way this looks. I did not even know you could add interactive widgets to the status bar. I agree that hours aren't really necessary, however I would use the format I would also remove the separator between the button and the frames/timecode text to help emphasize that the button is for that segment. I think the button might also be better on the right, since that mimics the location of arrows on a combo box. That is OS-specific however and there is something nice about the button not moving when you change settings, so probably just leave it unless everyone else things the right side is better. |
|
I've replaced the last colon with a period. It looks good. |
|
@scribblemaniac @davidlamhauge Excuse me, but that shouldn't be changed, the semicolon is proper in this case, please read this quote from the wiki on SMPTE Timecode.
Since Pencil2D is not dropping frames (we don't use floating point FPS) it should use the colon previous to the last commit. You also see this in programs like After Effects: |
|
@Jose-Moreno I know that colon is the 'proper' way to separate in timecode. That's why I used colon in the first case. |
|
Nice to see this implemented! I don’t have the time to have a thorough look at this right now, but based on the screenshot there’s one tiny detail that I would prefer to be changed (other than what’s been discussed so far), namely the alignment. The convention for status bars is to put permanent stuff on the right and temporary stuff or contents that change often on the left. Left-aligning the zoom level made sense previously because it was only a “temporary” message (which was simply very rarely overwritten), but now with the truly permanent widgets the right side would be a better fit. The reason I’m bringing this up is because I also got some status bar improvements in the works which make use of the left area (and also an interactive zoom widget, which I believe someone mentioned above). Nice work David! |
|
Fair point about the drop frame rule, however I should point out that |
J5lx
left a comment
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.
Time for a review! A few smaller issues, but nothing too serious. As a general note, I’d prefer something more descriptive like “timecode format” over “timecode text (level)” or “timecode label enum” in the code.
For the timecode format, I agree more with the use of a colon. I’m not sure if I’ve ever seen a dot used for that in the context of animation, so chances are our users too are going to encounter the colon notation elsewhere. I’d say it’s better to get used to that early – once you know the meaning, the dot notation isn’t much of an improvement anyway (IMO).
Lastly, in the original issue it was recommended to include both the SMPTE and the FFF (S:ff) timecode as options. Considering the latter’s use in Japanese animation, it seems worth adding that to me.
|
I think I have addressed all issues now. |
|
@davidlamhauge I think it's fine with what you have. Perhaps that japanese counter shown before was overly specific to some studios so it's not a problem. If you see Opentoonz Timecode Iwa Effect they only have absolute frames and SMPTE (which is widely used in Japan and the Americas along NTSC) |
|
I have included the Japanese S:FF mode, and moved the info to the timeline, so we have four options: No text, Frames, SMPTE and SFF. |
J5lx
left a comment
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.
I’m still unhappy about the implementation of the format selection menu and the TimeControls layout can use some tweaks. The other comments are basically just about cleanup, nothing serious. Once those things are taken care of, this PR is good to go!
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.
Here’s what I meant wrt the menu.
Edit: GitHub just had to mess up the order of my comments. This one was meant to be go along with the code suggestions above.
Co-authored-by: Jakob Gahde <j5lx@fmail.co.uk>
Co-authored-by: Jakob Gahde <j5lx@fmail.co.uk>
Co-authored-by: Jakob Gahde <j5lx@fmail.co.uk>
Co-authored-by: Jakob Gahde <j5lx@fmail.co.uk>
Co-authored-by: Jakob Gahde <j5lx@fmail.co.uk>
…ge/pencil into 935_statusbar_update
Hide timecode label when "No Text" is selected
J5lx
left a comment
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.
LGTM, merging.
|
@Jose-Moreno @candyface Shall we treat this as a fix for #935 as suggested in the OP or leave that issue open until we have an actual on-canvas display? |
|
Hmm.. It's not quite the solution I had in mind for #935 but it does solve the problem. Alright let's close it. |
|
@J5lx I feel the same as CandyFace. I suppose we can close it then. I'll do it after organizing the Issue via the project board to be related to this PR then. Thanks to everyone involved in this PR. |
This PR adds a label on the statusbar, that shows (1) No text, (2) Frame number or (3) Time code, for the scene.

I've put a ToolButton in the statusbar as well, so you can change your info type on the fly. The preferred info type is saved in the preferences.
For Time code, I've only MM:SS:FF and not Hours. Since we can have maximum 9999 frames in a scene, and 9999 frames at 3 fps equals less than an hour (55 minutes), I reckon that Hours is not needed, but it can easily be added.
I realize that this solution differs somewhat from the feature suggested in 935, but if it is accepted it addresses #935