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

Horizontal timeline option in Xsheet (issue#503) #1019

Merged
merged 96 commits into from Jul 20, 2017
Merged

Horizontal timeline option in Xsheet (issue#503) #1019

merged 96 commits into from Jul 20, 2017

Conversation

jaros3
Copy link
Contributor

@jaros3 jaros3 commented Feb 1, 2017

Overview

This feature adds a "toggle orientation" button to the Xsheet, letting you switch between default Xsheet layout (with vertical timeline) and compact horizontal timeline:

vertical

horizontal2

I've tested as many features as I could, though I'm no veteran in OpenToonz. I've got the basics of selecting, resizing, moving of columns and cells covered. I could not reproduce several more obscure column types; these will behave correctly in old layout and will need tweaks for the new orientation.

Implementation

OpenToonz code is strongly tied to vertical timeline; it maps X to column number and Y to row number. The link is pervasive in the code. In order to untie them, I introduced several concepts:

  • I recognize that there are Frame Axis and Layer Axis.
  • Frame Axis corresponds to Y, and Layer Axis corresponds to X in classic layout. They swap roles in the other layout.
  • Which axis goes where is a matter of View presenting the data. The data is stored exactly as it was.
  • Code makes excessive use of terms "row" and "column". I could not replace it everywhere; instead, I introduced a rule that "row" should be understood to mean Frame Axis and "column" to mean Layer Axis (always!)
  • A cell is identified by the pair of (frame, layer). This is called CellPosition.
  • Rather than map x <-> layer and y <-> frame separately, I map pairs (x, y) <-> (frame, layer).
  • The Column classes keep their name, and should be understood to mean Layer (synonymous: Level).
  • There is a class responsible for mapping CellPosition to screen XY or vice versa. The class is Orientation.
  • Orientation is responsible for everything layout related.
  • Xsheet remembers its orientation and can switch it.

Orientation operates on slightly higher level abstractions than the old code. Wherever possible, I used QRects to mark rectangular areas with meaningful role.

Old code

  int x      = m_viewer->columnToX(col);
  int y      = m_viewer->rowToY(row);
  QRect rect = QRect(x + 1, y + 1, ColumnWidth - 1, RowHeight - 1);

New code

  QPoint xy      = m_viewer->positionToXY(CellPosition(row, col));
  int x          = xy.x();
  int y          = xy.y();
  QRect cellRect = o->rect(PredefinedRect::CELL).translated(xy);
  QRect rect     = cellRect.adjusted(1, 1, -1, -1);

I keep x and y for compatibility; however, whenever possible, I prefer higher level abstraction of QPoint xy. QRect arithmetics rewards this approach. Sometimes QLine is more appropriate; and sometimes I fall back to X and Y calculations, termed as Layer Axis and Frame Axis. Orientation helps me translate them to screen XY in the end.

Orientation provides quite a few enumerated QRects, QLines, ints and QPainterPaths to satisfy any needs in marking up the screen. These can be retrieved via methods similar to Orientation::rect ():

enum class PredefinedRect {
  CELL,                     //! size of a cell
  DRAG_HANDLE_CORNER,       //! area for dragging a cell
  KEY_ICON,                 //! position of key icon
  CELL_NAME,                //! cell name box
  CELL_NAME_WITH_KEYFRAME,  //! cell name box when keyframe is displayed
  END_EXTENDER,             //! bottom / right extender
  BEGIN_EXTENDER,           //! top / left extender

These are almost always relative to the top left corner of corresponding cell. These primitives are used for both displaying the items and responding to mouse events:

Old code:

    if (column && column->getSoundColumn()) {
      // sound column
      if (x > 20 && 3 * RowHeight + 5 <= y && y < 3 * RowHeight + 33)
        m_tooltip = tr("Click to play the soundtrack back");
      else if (x >= 10 && x <= 20 && RowHeight + RowHeight / 2 < y &&
               y < 8 * RowHeight - RowHeight / 2)
        m_tooltip = tr("Set the volume of the soundtrack");
    }

New code:

    if (column && column->getSoundColumn()) {
      // sound column
      if (o->rect(PredefinedRect::SOUND_ICON).contains(mouseInCell))
        m_tooltip = tr("Click to play the soundtrack back");
      else if (o->rect(PredefinedRect::VOLUME_AREA).contains(mouseInCell))
        m_tooltip = tr("Set the volume of the soundtrack");
    }

which, among other things, reduces chances for inconsistencies.

Now screen (x, y) of various Xsheet UI items depend on both (row, col).
Row means frame number; column means layer number. Keeping old terms.
Orientation class is responsible for mapping timeline either horizontally or vertically.
Size of cell varies in vertical and horizontal layout
Dropped right to left orientation
Splitting too long draw cells into several functions
Orientation can now produce lines
Preparing to display drag handle and locked dotted line
Improved drawing of sound cell
Drawing drag handle on the top side of the cell
Drawing locked dotted line on the top side
Factored out similarities of drawing various kinds of cells
Refactored it as a dimension
Orientation also provides painter paths
Fixed an upper extender crash
Moved orientation to toonzlib
Xsheet now features a flip orientation button.
When multiple Xsheet windows are open, flip is slightly buggy due to use of shared columns fan.
Xsheet now stores a column fan for each orientation
XsheetViewers view mode can be toggled independently
Sound was too wide in vertical mode
Resized cell in horizontal mode.
Added precautions against nullptr orientation and other fields.
Scrolling appropriate pane in both layouts.
Fixed highlighting area
Fixed scroll bar regression by making disconnect more specific
Adjusted display of markers
Adjusted drawing onion in both layouts
There was a minor issue with misbehaving dragging Onion handle up in horizontal layout.
@ghost
Copy link

ghost commented Jun 24, 2017

@Agnyy I saw the video on YouTube showing some of them already implemented. Is that code available?

@Agnyy
Copy link

Agnyy commented Jun 24, 2017

@turtleTooth The programmer had to lay out. Here you can look it up. If you do not find it then let me tell the programmer to send the information. https://github.com/jaros3/opentoonz/tree/feature/timeline-func

@manongjohn
Copy link
Collaborator

@shun-iwasawa
I finally figured out why I was seeing a performance issue. This is due to my computer.

I forgot that I had to force the official OT releases in the normal installation folder to use GPU in order to run decently on my system. For testing purposes, I was running right out of the Release/RelWithDebInfo folders after building which wasn't set up to use GPU and therefore ran badly.

After setting the builds in my dev environments to use GPU, I see no issue with performance.

@ghost
Copy link

ghost commented Jul 8, 2017

@manongjohn Thank you for all your work with this. You are awesome!

I am playing with getting text levels working again in OpenToonz. (They started out only for Magpie Pro files, but I am working at expanding their functionality.)

Could you load this scene and see how it looks in the horizontal timeline?
text.zip

It looks like this in vertical:
vista

@manongjohn
Copy link
Collaborator

@turtleTooth I saw there was a column type of "SoundText" that appeared to be sourced from Magpie files. I didn't have one to test and I couldn't make one so I gave up on it. It was the only column type I couldn't check/test. I had hoped the layout was going to be ok as is. Apparently, it isn't.

After loading your sample file, the column header in both vertical and horizontal layouts need a lot of work. It seems the layout is still from Toonz Harmony? I'm guessing the SoundText layout was never updated to be in line with the other columns in OpenToonz. I also have to fix how it loads into the cell area in horizontal mode.

Now that I have an example of one, I will work on fixing it to match the rest of OpenToonz. :)

@ghost
Copy link

ghost commented Jul 8, 2017

Doh! I'm sorry for the extra work. I had just seen a bunch of requests related to text columns and thought I would look into it.

@manongjohn
Copy link
Collaborator

@turtletooh No problem. Felt rather incomplete no being able to test it all. Now I can. :) Shouldn't take me very long.

@2dvision
Copy link

2dvision commented Jul 9, 2017

Is it possible:

  • Fill gap by shortcut? Toon boom harmony and Adobe animate have "F5" shortcut for fill gap.
    fill gap
  • Ability to add camera to timeline, like a level? At the moment, for animate camera need only use function editor.
    toon boom camera

@ghost
Copy link

ghost commented Jul 9, 2017

@2dvision i don't think adding timeline exclusive functionality makes alot of sense, these features probably need their own issue.

@2dvision
Copy link

@tGuMedia Ok, I thought these could be a few optimizations for the timeline, not feature requests.

@artisteacher
Copy link
Contributor

@2dvision while you are waiting for additional camera/timeline functionality, the camera can also be animated directly in the viewer using the edit tool.

Select the camera, choose the mode (position, scale or etc.), make adjustments/edits as desired
and use the keyframe button in the bottom of the viewer to set keyframes at the desired frame numbers.
screen shot 2017-07-10 at 11 53 42 am
screen shot 2017-07-10 at 11 56 54 am
It's not quite what you are looking for, but at least you are not limited to the function editor.

@2dvision
Copy link

@artisteacher Thank you. I found a trick for animate camera in Xsheet too.

@morevnaproject
Copy link
Contributor

This feature is now included into our builds ^__^ - https://morevnaproject.org/2017/07/17/horizontal-timeline-now-available-opentoonz/

@manongjohn
Copy link
Collaborator

@morevnaproject I just submitted a commit to this PR. If you can, you might want to pull the latest as it includes some Timeline SoundText columns and tool tips fixes that needed to be made. I've also added a preference setting to show/hide Column number in Timeline mode as well as Ctrl-Click of buttons in Xsheet mode will do a Toggle All of that button.

@shun-iwasawa I've decided to freeze the code for this PR pending any major bugs found or additional changes you might require of it. I do not plan on modifying this PR any more beyond that. The latest commit does not include the UI changes we've been discussing. I will do that as a separate PR since I don't know when that will ever be finalized.

Having said that, please review when you have time. Hopefully it can be merged sometime soon.

Once this is merged my plans are as follows:

  1. Sync these changes to the @jaros3's feature/timeline-func branch
    This branch is quite far behind and I will need to merge all my changes into it. It's going to be a pain to do but must be done in order for continued work on it..
  2. Reorder timeline columns to be bottom-up, in line with other software
    The feature/timeline-func branch has some code that I think I can use to help reorder the columns which is why I want to merge everything as soon as I can. I'd like to have it reversed by the next official release.
  3. Review the feature/timeline-func functionality and see what I can do to further it along.

@shun-iwasawa
Copy link
Member

Jenkins

@2dvision
Copy link

What do you think about replace icons with checkbox in timeline?

timeline checkbox

@beeheemooth
Copy link

Dots

@morevnaproject
Copy link
Contributor

@manongjohn Thank you! We will include it into the next build. ^__^

@shun-iwasawa
Copy link
Member

I confirmed that it works great for now.
Thank you very much @manongjohn and @jaros3 for this big improvement!

I found a tiny problem that the xsheet and the function editor do not scroll synchronously. I'll post the separate PR for it.

LGTM

@shun-iwasawa shun-iwasawa merged commit 203cc81 into opentoonz:master Jul 20, 2017
@morevnaproject
Copy link
Contributor

Great! Also, much thanks to @Agnyy for coordination and funding the work of @jaros3!

@shun-iwasawa
Copy link
Member

Yes, of course! Thank you @Agnyy for your help!

@ghost
Copy link

ghost commented Jul 25, 2017

Thank you all. @manongjohn Thank you for picking this up and running with it. You have done a great job!

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

Successfully merging this pull request may close these issues.

None yet