Running under Wayland reveals bad rendering performance

[michaelgregorius]

System Information

Linux - Wayland

LMMS Version(s)

bda1a9c

Bug Summary

Short story: the Wayland rendering loop reveals performance problems if an application is not able to update its GUI at a rate of 60 Hz. Frames that take longer than 1/60 second to render will be dropped which to the users appears like very laggy updates in the seconds range.

With intelligent data structures and efficient rendering code it should be possible to reach the requested update rate.

This will mostly affect custom widgets like the waveform renderer/view.

Known affected components

• Waveform renderer: See this comment and the ones following for now: SlicerT UI update #7453 (comment)
• Song Editor: see this comment in this issue: Running under Wayland reveals bad rendering performance #7462 (comment)

Expected Behaviour

Render quickly.

• I have searched all existing issues and confirmed that this is not a duplicate.

Edit: I have added @cyberrumor's findings.

[cyberrumor]

I've bisected this. Performance is fine until commit ce17c956368789d6add77c57bba5e0a8bb63942a, at which point performance tanks.

[michaelgregorius]

Linking the aforementioned commit directly: ce17c95. It's related to "Add Continuous Auto-Scrolling" (#7396). However, I assume that this problem is not really related to Wayland which is what this issue is about.

[cyberrumor]

Acknowledged. I'm also on Wayland and experience dropped frames when scrolling via mouse+horizontal_scrollbar in the SongEditor. I note that you experience this in custom widgets instead. On Xorg, this is much smoother.

On further consideration, I suppose my bisect has identified a commit which caused the bug you filed to manifest in a new situation, which is not the same as identifying the commit which caused slow rendering in general. Apologies for the noise.

[michaelgregorius]

@cyberrumor, no need to apologize as it seems that I was too quick in my judgment.

You are right, your bisect has found another place where the slow rendering of custom widgets manifests as slow rendering with dropped frames under Wayland. I will create a list of known problems in my original description and add your finding there as well.

[michaelgregorius]

I have profiled the behavior of the Song Editor with auto-scroll turned on and playing Greippi - Krem Kaakkuja (Second Flight Remix).mmpz.

The result is that the calls are dominated by Song::updateLength(). It is called by Clip::changeLength(const TimePos& length) which in turn is called by TrackContentWidget::changePosition(const TimePos& newPos). The latter contains the following line:

lmms/src/gui/tracks/TrackContentWidget.cpp

Line 273 in 07baf9e

clip->changeLength( clip->length() );

Removing this line has no side effects for me and it also removes the calls to Song::updateLength() from the top of the calls. After this change AutomationClipView::paintEvent(QPaintEvent *) is at the top for the results of the lmms object.

Please note that the aforementioned line of code is bad style anyway. Normally that call should do nothing because the size of the clip is not changed. However, Clip::changeLength always sets the length and calls for an update of the song length:

lmms/src/core/Clip.cpp

Lines 114 to 119 in 07baf9e

	void Clip::changeLength( const TimePos & length )
	{
	m_length = length;
	Engine::getSong()->updateLength();
	emit lengthChanged();
	}

So the code in TrackContentWidget does not really make that call because it wants to actually update the length but because it is or was interested in some side effects of that call. However, in that case the code should state directly what it is actually interested in.

Notes on profiling

To profile an application with Valgrind from a certain point on the profiling session must be started with the parameter instr-atstart being set to no. Example:

valgrind --tool=callgrind --instr-atstart=no ./build/lmms

In another console the collection can then be started with

callgrind_control -i on

and turned off again with

callgrind_control -i off

It's also possible to pass the PID to the callgrind_control command if there are several active callgrind sessions.

Profiler results

Before removing the call to Clip::changeLength: callgrind.out.16727.zip

After removal: callgrind.out.18024.zip

[michaelgregorius]

I have created pull request #7570 which removes the call clip->changeLength( clip->length() ).

[cyberrumor]

Interesting, my investigation had led me to SongEditor.cpp::scrolled. The emit PositionChanged(m_currentPosition = TimePos(new_pos) appeared to take an extremely long time compared to the rest of stuff in that file. I didn't look at other files though, and I'm not sure what happens when that signal is emitted.

I pulled your change down and poked around with it. Interface is still a bit laggy with that horizontal scrollbar but I don't notice anything misbehaving as far as clip length.

[michaelgregorius]

@cyberrumor, it looks like that signal leads to the updates described above. TrackContainerView::addTrackView is the method where the TrackContentWidget of each added TrackView is connected to the positionChanged signal of the TrackContainerView, i.e. SongEditor:

lmms/src/gui/editors/TrackContainerView.cpp

Lines 155 to 164 in 07baf9e

	TrackView * TrackContainerView::addTrackView( TrackView * _tv )
	{
	m_trackViews.push_back( _tv );
	m_scrollLayout->addWidget( _tv );
	connect( this, SIGNAL( positionChanged( const lmms::TimePos& ) ),
	_tv->getTrackContentWidget(),
	SLOT( changePosition( const lmms::TimePos& ) ) );
	realignTracks();
	return( _tv );
	}