Page MenuHome

VSE: Change grid line drawing
ClosedPublic

Authored by Richard Antalik (ISS) on Jul 3 2021, 12:44 AM.

Details

Summary

Add overlay option to disable grid drawing.
Reuse drawing code from other editors (timeline editor)
Add argument display_minor_lines to function UI_view2d_draw_lines_x__discrete_frames_or_seconds
This way minor line drawing can be disabled and so it doesn't cause too much visual noise. Also spacing seems to be too fine so it uses 3x what is defined in preferences.

Diff Detail

Repository
rB Blender
Branch
arcpatch-D11790 (branched from master)
Build Status
Buildable 15664
Build 15664: arc lint + arc unit

Event Timeline

Richard Antalik (ISS) requested review of this revision.Jul 3 2021, 12:44 AM
Richard Antalik (ISS) created this revision.
Richard Antalik (ISS) edited the summary of this revision. (Show Details)Jul 3 2021, 12:49 AM

Hi, as a frequent user of the VSE, I don't find them distracting, but rather the opposite as you don't have to make guess work to know if the strips are aligned with the time. What I'd do instead (kind of feature/improvement request) is to draw lines only for the text being displayed (for example 5+00, 5+01, etc.). This way it won't become as crammed as it does now.
Of course, making the lines opcional is an even better solution as it lets you choose.

The industry standard is time indication lines/ticks in the ruler. Even in open source projects:

Tactview:

Shotcut:

Kdenlive:

Olive:

Flowblade:

When in doubt about "Industry standards" visit the Kdenlive handbook: https://kdenlive.org/en/video-editing-applications-handbook/

So instead of more options, maybe it could be considered to move the Timeline time lines up into the ruler where the time values are?

Thanks for feedback,

Using grid lines from timeline editor would be definitely improvement, and should do what @Gerard Taulats Braos (Tabra) suggested.
Drawing ticks in scrubbing area is a bit problematic, because this wouldn't play nice with other editors like graph editor.

As mentioned here: https://developer.blender.org/T89626 there are plenty of inconsistencies between the animation editors. But the one size fits all kind of thinking, may not be good for users in all cases. Ex.some of the animation editors, could benefit from using ticks ex. the NLA and the VSE.

Yes, the grid should be an overlay option and not be completely removed forever. For the case when the grid is off, not having any vertical indicator except for the playhead is acceptable (most software does not expose the current time in the playhead). The suggestion of showing vertical lines only to match the visible number is good, I would be curious to see it.

The suggestion of showing vertical lines only to match the visible number is good, I would be curious to see it.

Timeline editor:

  • Add grid overlay setting, reuse more common grid drawing code
Richard Antalik (ISS) edited the summary of this revision. (Show Details)Jul 7 2021, 4:52 AM
Richard Antalik (ISS) added a comment.EditedJul 7 2021, 5:07 AM

I think this last alternating color of grid lines and channels looks very unpleasing. I also don't like that grid lines that look like major are inbetween the ticks on scrubbing area.

After unsuccesfully playing with theme, I think that only solution is to draw major grid lines only. I guess this will mean to fix code that is removed in this patch.

Richard Antalik (ISS) planned changes to this revision.Jul 7 2021, 5:09 AM
This comment was removed by Peter Fog (tintwotin).
Richard Antalik (ISS) edited the summary of this revision. (Show Details)Jul 7 2021, 7:22 AM
Peter Fog (tintwotin) added a comment.EditedJul 7 2021, 8:01 AM

Major vertical lines only, but the horizontal lines are on the same theme color:

Removing the horizontal lines and theming the values lines darker than horizontal bars(making them lighter will add too much noise):

A note. The advantage of having the ticks in the "value ruler" is that they're visible, when strips are covering the sequencer area(background drawing):

Richard Antalik (ISS) edited the summary of this revision. (Show Details)
  • Display only major ticks, update theme, so appearence is identical to previous version
Richard Antalik (ISS) edited the summary of this revision. (Show Details)Jul 14 2021, 4:47 PM

Is the vertical and the horizontal lines still the same theme color? If they are, imo it is very problematic - see my previous post above.

I'm not sure why the vertical time lines should become overlay options? I can't think of any workflow where you would need to switch them on and off?
Imo, it's more of a theming thing, and I guess users/themers should be allowed to make those transparent instead?

@Francesco Siddi (fsiddi) Checked with darker lines, not sure if I like it more than what I proposed

I think, what this needs is to increase space between the lines to be less visually disturbing, but this is global setting for all editors.

Something like this:

This ugly hack though:

U.v2d_min_gridsize *= 3;
UI_view2d_draw_lines_x__discrete_frames_or_seconds(
    v2d, scene, (sseq->flag & SEQ_DRAWFRAMES) == 0, false);
U.v2d_min_gridsize /= 3;
  • Use darker lines, Change grid line spacing

Inconsistencies of the current 3.0 alpha between the VSE and other editors:


Ex. the Spreadsheet do not have thin horizontal lines, and a less contrast in the wide horizontal lines(looking much better).
Ex. the Spreadsheet do not have the row numbers written rotated -90 degrees.
Ex. the Spreadsheet row numbers have a column of their own - which doesn't scroll.
Ex. the Spreadsheet have row/channel headers.
Ex. the Spreadsheet and the Timeline editor have dark lines.
Ex. Timeline have major and minor vertical lines.

Richard Antalik (ISS) retitled this revision from VSE: Don't draw time grid lines to VSE: Change grid line drawing.Jul 28 2021, 12:39 PM
Richard Antalik (ISS) edited the summary of this revision. (Show Details)

These are nice improvements in my opinion. Good to see the VSE getting a bit more attention on the aesthetics, where it's clearly lacking behind (like in many other parts).
But I'll let @Francesco Siddi (fsiddi) give his feedback first.

Looking over the code quickly, things look (almost) good.

source/blender/makesrna/intern/rna_space.c
5555

Please add a description for the option.

Richard Antalik (ISS) marked an inline comment as done.
  • Add a description for show_grid property

These are nice improvements in my opinion. Good to see the VSE getting a bit more attention on the aesthetics, where it's clearly lacking behind (like in many other parts).
But I'll let @Francesco Siddi (fsiddi) give his feedback first.

Yesterday he checked experimental build, said it looks OK

Tested it and it works as expected. Thank you for working on this.

This revision is now accepted and ready to land.Jul 28 2021, 3:33 PM
This revision was automatically updated to reflect the committed changes.

What is the point of having the horizontal and vertical lines the same theme color?

They have two different purposes. And why do this theme color not have alpha?

When switching off Grid in Overlay options, it is only switching off the vertical lines, so is Grid really the correct word for the time indication lines?

Btw. having a NLE timeline/work-area divided into squares by a grid, is a quite unorthodox. Normally the work area is completely empty, with ticks in a ruler and maybe a line between tracks or maybe nothing.

What is the point of having the horizontal and vertical lines the same theme color?

They have two different purposes. And why do this theme color not have alpha?

Can't really say why. I guess this could be changed

When switching off Grid in Overlay options, it is only switching off the vertical lines, so is Grid really the correct word for the time indication lines?

Hmm this is good point, Both lines should be hidden.

Having said that, we want to add channel headers, so there is good chance these lines will be removed, and if not they won't be part of grid in theme probably.

I think the "grid" should be separated into "channel separation lines" and "time indication lines", and have separate theme colors which includes alpha(so they can be themed "away").

The channel separation lines could ex. be used like this: