Discussion:
[Audacity-devel] TrackPanel Resize Bug
Darrell Walisser
2016-09-11 14:33:30 UTC
Permalink
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.

The reason for lag appears to be a flood of calls to
TrackArtist::UpdateVRuler originating from here:

void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();

UpdateVRuler(t);

e.Skip();
}

If the call to UpdateVRuler here is removed, the problem disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.


In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
James Crook
2016-09-11 15:58:01 UTC
Permalink
Thanks Darrell.

Fixed now? See:
https://github.com/audacity/audacity/commit/4251ef8c48e53d269fde7d5f0f55ca74bd663a76

It seemed to be:
mRuler->Refresh();
in void TrackPanel::UpdateVRulerSize() ( in turn called from
UpdateVRuler)

being called too often. That line is responsible for updating the
HORIZONTAL ruler, in the case that the changes in the vertical ruler
changed the width of the vertical ruler (e.g. by causing longer numbers
to be displayed than before). You get that when you zoom in on the
vertical ruler by clicking on it, for example. However it was also being
called for a vertical ruler height change, which should NOT affect the
horizontal ruler.

--James.
Post by Darrell Walisser
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.
The reason for lag appears to be a flood of calls to
void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();
UpdateVRuler(t);
e.Skip();
}
If the call to UpdateVRuler here is removed, the problem disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.
In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
------------------------------------------------------------------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Darrell Walisser
2016-09-13 16:06:42 UTC
Permalink
Not fixed. If I completely remove UpdateVRulerSize there is no change.
Post by James Crook
Thanks Darrell.
https://github.com/audacity/audacity/commit/4251ef8c48e53d269fde7d5f0f55ca
74bd663a76
mRuler->Refresh();
in void TrackPanel::UpdateVRulerSize() ( in turn called from
UpdateVRuler)
being called too often. That line is responsible for updating the
HORIZONTAL ruler, in the case that the changes in the vertical ruler
changed the width of the vertical ruler (e.g. by causing longer numbers to
be displayed than before). You get that when you zoom in on the vertical
ruler by clicking on it, for example. However it was also being called for
a vertical ruler height change, which should NOT affect the horizontal
ruler.
--James.
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.
The reason for lag appears to be a flood of calls to
void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();
UpdateVRuler(t);
e.Skip();
}
If the call to UpdateVRuler here is removed, the problem disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.
In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
------------------------------------------------------------------------------
_______________________________________________
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Darrell Walisser
2016-09-13 16:21:24 UTC
Permalink
Here is a trace of TrackPanel::UpdateVRuler, as you can see almost all of
them come from the resize event.

https://gist.github.com/walisser/f2cc1a265e0d8bed03a9de54e560c1e0

On Tue, Sep 13, 2016 at 12:06 PM, Darrell Walisser <
Post by Darrell Walisser
Not fixed. If I completely remove UpdateVRulerSize there is no change.
Post by James Crook
Thanks Darrell.
https://github.com/audacity/audacity/commit/4251ef8c48e53d26
9fde7d5f0f55ca74bd663a76
mRuler->Refresh();
in void TrackPanel::UpdateVRulerSize() ( in turn called from
UpdateVRuler)
being called too often. That line is responsible for updating the
HORIZONTAL ruler, in the case that the changes in the vertical ruler
changed the width of the vertical ruler (e.g. by causing longer numbers to
be displayed than before). You get that when you zoom in on the vertical
ruler by clicking on it, for example. However it was also being called for
a vertical ruler height change, which should NOT affect the horizontal
ruler.
--James.
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.
The reason for lag appears to be a flood of calls to
void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();
UpdateVRuler(t);
e.Skip();
}
If the call to UpdateVRuler here is removed, the problem disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.
In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
------------------------------------------------------------------------------
_______________________________________________
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Darrell Walisser
2016-09-13 16:43:28 UTC
Permalink
Alternate fix, took a guess that maybe TrackArtist::UpdateVRuler was
triggering a resize event. Maybe hints what the root cause is. Not sure
what to do with the experimental part, looks to me like it does a second
redundant update on the first track.

void TrackPanel::UpdateTrackVRuler(Track *t)
{
wxASSERT(t);
if (!t)
return;

std::vector<Track*> tracks;
std::vector<wxRect> rects;

tracks.push_back(t);
rects.push_back( wxRect(GetVRulerOffset(),
kTopMargin,
GetVRulerWidth(),
t->GetHeight() - (kTopMargin + kBottomMargin)));

Track *l = t->GetLink();
if (l)
{
tracks.push_back(l);
wxRect rect = rects[0];
rect.height = l->GetHeight() - (kTopMargin + kBottomMargin);
rects.push_back(rect);
}

#ifdef EXPERIMENTAL_OUTPUT_DISPLAY
else if(MONO_WAVE_PAN(t)){
rect.height = t->GetHeight(true) - (kTopMargin + kBottomMargin);
mTrackArtist->UpdateVRuler(t, rect);
}
#endif

for (int i = 0; i < tracks.size(); i++)
mTrackArtist->UpdateVRuler(tracks[i], rects[i]);
}

On Tue, Sep 13, 2016 at 12:21 PM, Darrell Walisser <
Post by Darrell Walisser
Here is a trace of TrackPanel::UpdateVRuler, as you can see almost all of
them come from the resize event.
https://gist.github.com/walisser/f2cc1a265e0d8bed03a9de54e560c1e0
On Tue, Sep 13, 2016 at 12:06 PM, Darrell Walisser <
Post by Darrell Walisser
Not fixed. If I completely remove UpdateVRulerSize there is no change.
Post by James Crook
Thanks Darrell.
https://github.com/audacity/audacity/commit/4251ef8c48e53d26
9fde7d5f0f55ca74bd663a76
mRuler->Refresh();
in void TrackPanel::UpdateVRulerSize() ( in turn called from
UpdateVRuler)
being called too often. That line is responsible for updating the
HORIZONTAL ruler, in the case that the changes in the vertical ruler
changed the width of the vertical ruler (e.g. by causing longer numbers to
be displayed than before). You get that when you zoom in on the vertical
ruler by clicking on it, for example. However it was also being called for
a vertical ruler height change, which should NOT affect the horizontal
ruler.
--James.
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.
The reason for lag appears to be a flood of calls to
void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();
UpdateVRuler(t);
e.Skip();
}
If the call to UpdateVRuler here is removed, the problem disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.
In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
------------------------------------------------------------------------------
_______________________________________________
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Paul Licameli
2016-09-13 18:58:16 UTC
Permalink
I don't see what this rewrite of that function is meant to accomplish.

The event is generated in the function TrackList::ResizedEvent and only
there. That, in turn, is called by functions that change the members of
the list, but also some others, including Track::SetHeight which is the
likely source of events in this case.

I can understand why there are two calls to SetHeight instead of one with
each mouse motion event when you drag the separator of channels or the
bottom edge of a stereo track. I don't understand why this doubling makes
a really huge difference, or why it doesn't happen with waveform view. (Or
did you not mean to imply that it doesn't.)

Still you might examine all uses of SetHeight and figure out how to lift
the resize events out, and avoid the duplication.


On Tue, Sep 13, 2016 at 12:43 PM, Darrell Walisser <
Post by Darrell Walisser
Alternate fix, took a guess that maybe TrackArtist::UpdateVRuler was
triggering a resize event. Maybe hints what the root cause is. Not sure
what to do with the experimental part, looks to me like it does a second
redundant update on the first track.
void TrackPanel::UpdateTrackVRuler(Track *t)
{
wxASSERT(t);
if (!t)
return;
std::vector<Track*> tracks;
std::vector<wxRect> rects;
tracks.push_back(t);
rects.push_back( wxRect(GetVRulerOffset(),
kTopMargin,
GetVRulerWidth(),
t->GetHeight() - (kTopMargin + kBottomMargin)));
Track *l = t->GetLink();
if (l)
{
tracks.push_back(l);
wxRect rect = rects[0];
rect.height = l->GetHeight() - (kTopMargin + kBottomMargin);
rects.push_back(rect);
}
#ifdef EXPERIMENTAL_OUTPUT_DISPLAY
else if(MONO_WAVE_PAN(t)){
rect.height = t->GetHeight(true) - (kTopMargin + kBottomMargin);
mTrackArtist->UpdateVRuler(t, rect);
}
#endif
for (int i = 0; i < tracks.size(); i++)
mTrackArtist->UpdateVRuler(tracks[i], rects[i]);
}
On Tue, Sep 13, 2016 at 12:21 PM, Darrell Walisser <
Post by Darrell Walisser
Here is a trace of TrackPanel::UpdateVRuler, as you can see almost all of
them come from the resize event.
https://gist.github.com/walisser/f2cc1a265e0d8bed03a9de54e560c1e0
On Tue, Sep 13, 2016 at 12:06 PM, Darrell Walisser <
Post by Darrell Walisser
Not fixed. If I completely remove UpdateVRulerSize there is no change.
Post by James Crook
Thanks Darrell.
https://github.com/audacity/audacity/commit/4251ef8c48e53d26
9fde7d5f0f55ca74bd663a76
mRuler->Refresh();
in void TrackPanel::UpdateVRulerSize() ( in turn called from
UpdateVRuler)
being called too often. That line is responsible for updating the
HORIZONTAL ruler, in the case that the changes in the vertical ruler
changed the width of the vertical ruler (e.g. by causing longer numbers to
be displayed than before). You get that when you zoom in on the vertical
ruler by clicking on it, for example. However it was also being called for
a vertical ruler height change, which should NOT affect the horizontal
ruler.
--James.
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.
The reason for lag appears to be a flood of calls to
void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();
UpdateVRuler(t);
e.Skip();
}
If the call to UpdateVRuler here is removed, the problem disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.
In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
------------------------------------------------------------------------------
_______________________________________________
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
James Crook
2016-09-13 19:43:54 UTC
Permalink
Post by Paul Licameli
I don't see what this rewrite of that function is meant to accomplish.
I can see why it might make a difference.

I think it may be to do with the resizing of the second track in a
stereo pair.
It might not correctly know what size it should be, and the reordering
could fix that. BUT I need to actually try the modified and unmodified
code to be sure, and haven't yet got around to that.

--James.
Post by Paul Licameli
The event is generated in the function TrackList::ResizedEvent and only
there. That, in turn, is called by functions that change the members of
the list, but also some others, including Track::SetHeight which is the
likely source of events in this case.
I can understand why there are two calls to SetHeight instead of one with
each mouse motion event when you drag the separator of channels or the
bottom edge of a stereo track. I don't understand why this doubling makes
a really huge difference, or why it doesn't happen with waveform view. (Or
did you not mean to imply that it doesn't.)
Still you might examine all uses of SetHeight and figure out how to lift
the resize events out, and avoid the duplication.
On Tue, Sep 13, 2016 at 12:43 PM, Darrell Walisser <
Post by Darrell Walisser
Alternate fix, took a guess that maybe TrackArtist::UpdateVRuler was
triggering a resize event. Maybe hints what the root cause is. Not sure
what to do with the experimental part, looks to me like it does a second
redundant update on the first track.
void TrackPanel::UpdateTrackVRuler(Track *t)
{
wxASSERT(t);
if (!t)
return;
std::vector<Track*> tracks;
std::vector<wxRect> rects;
tracks.push_back(t);
rects.push_back( wxRect(GetVRulerOffset(),
kTopMargin,
GetVRulerWidth(),
t->GetHeight() - (kTopMargin + kBottomMargin)));
Track *l = t->GetLink();
if (l)
{
tracks.push_back(l);
wxRect rect = rects[0];
rect.height = l->GetHeight() - (kTopMargin + kBottomMargin);
rects.push_back(rect);
}
#ifdef EXPERIMENTAL_OUTPUT_DISPLAY
else if(MONO_WAVE_PAN(t)){
rect.height = t->GetHeight(true) - (kTopMargin + kBottomMargin);
mTrackArtist->UpdateVRuler(t, rect);
}
#endif
for (int i = 0; i < tracks.size(); i++)
mTrackArtist->UpdateVRuler(tracks[i], rects[i]);
}
On Tue, Sep 13, 2016 at 12:21 PM, Darrell Walisser <
Post by Darrell Walisser
Here is a trace of TrackPanel::UpdateVRuler, as you can see almost all of
them come from the resize event.
https://gist.github.com/walisser/f2cc1a265e0d8bed03a9de54e560c1e0
On Tue, Sep 13, 2016 at 12:06 PM, Darrell Walisser <
Post by Darrell Walisser
Not fixed. If I completely remove UpdateVRulerSize there is no change.
Post by James Crook
Thanks Darrell.
https://github.com/audacity/audacity/commit/4251ef8c48e53d26
9fde7d5f0f55ca74bd663a76
mRuler->Refresh();
in void TrackPanel::UpdateVRulerSize() ( in turn called from
UpdateVRuler)
being called too often. That line is responsible for updating the
HORIZONTAL ruler, in the case that the changes in the vertical ruler
changed the width of the vertical ruler (e.g. by causing longer numbers to
be displayed than before). You get that when you zoom in on the vertical
ruler by clicking on it, for example. However it was also being called for
a vertical ruler height change, which should NOT affect the horizontal
ruler.
--James.
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.
The reason for lag appears to be a flood of calls to
void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();
UpdateVRuler(t);
e.Skip();
}
If the call to UpdateVRuler here is removed, the problem disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.
In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
------------------------------------------------------------------------------
_______________________________________________
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
------------------------------------------------------------------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
James Crook
2016-09-13 20:52:01 UTC
Permalink
Latest change proposed to UpdateTrackVRule makes no difference for me on
Windows.
Spectrogram resizing with or without is laggy, but not ridiculously slow
(about a second between repaints, and they are not postponed until mouse
up).


The earlier proposal to remove the call to
UpdateVRuler(t);

Does introduce a bug that is visible for stereo spectrogram resizing but
not wave resizing. The vRuler does not resize to accommodate the
changed numerical frequency scale and numbers may be printed over the
spectrogram during resizing. Not an issue for wave view as you're not
getting extra digits during resizing the tracks.


--James.
Post by James Crook
Post by Paul Licameli
I don't see what this rewrite of that function is meant to accomplish.
I can see why it might make a difference.
I think it may be to do with the resizing of the second track in a
stereo pair.
It might not correctly know what size it should be, and the reordering
could fix that. BUT I need to actually try the modified and
unmodified code to be sure, and haven't yet got around to that.
--James.
Post by Paul Licameli
The event is generated in the function TrackList::ResizedEvent and only
there. That, in turn, is called by functions that change the members of
the list, but also some others, including Track::SetHeight which is the
likely source of events in this case.
I can understand why there are two calls to SetHeight instead of one with
each mouse motion event when you drag the separator of channels or the
bottom edge of a stereo track. I don't understand why this doubling makes
a really huge difference, or why it doesn't happen with waveform view. (Or
did you not mean to imply that it doesn't.)
Still you might examine all uses of SetHeight and figure out how to lift
the resize events out, and avoid the duplication.
On Tue, Sep 13, 2016 at 12:43 PM, Darrell Walisser <
Post by Darrell Walisser
Alternate fix, took a guess that maybe TrackArtist::UpdateVRuler was
triggering a resize event. Maybe hints what the root cause is. Not sure
what to do with the experimental part, looks to me like it does a second
redundant update on the first track.
void TrackPanel::UpdateTrackVRuler(Track *t)
{
wxASSERT(t);
if (!t)
return;
std::vector<Track*> tracks;
std::vector<wxRect> rects;
tracks.push_back(t);
rects.push_back( wxRect(GetVRulerOffset(),
kTopMargin,
GetVRulerWidth(),
t->GetHeight() - (kTopMargin + kBottomMargin)));
Track *l = t->GetLink();
if (l)
{
tracks.push_back(l);
wxRect rect = rects[0];
rect.height = l->GetHeight() - (kTopMargin + kBottomMargin);
rects.push_back(rect);
}
#ifdef EXPERIMENTAL_OUTPUT_DISPLAY
else if(MONO_WAVE_PAN(t)){
rect.height = t->GetHeight(true) - (kTopMargin + kBottomMargin);
mTrackArtist->UpdateVRuler(t, rect);
}
#endif
for (int i = 0; i < tracks.size(); i++)
mTrackArtist->UpdateVRuler(tracks[i], rects[i]);
}
On Tue, Sep 13, 2016 at 12:21 PM, Darrell Walisser <
Post by Darrell Walisser
Here is a trace of TrackPanel::UpdateVRuler, as you can see almost all of
them come from the resize event.
https://gist.github.com/walisser/f2cc1a265e0d8bed03a9de54e560c1e0
On Tue, Sep 13, 2016 at 12:06 PM, Darrell Walisser <
Post by Darrell Walisser
Not fixed. If I completely remove UpdateVRulerSize there is no change.
Post by James Crook
Thanks Darrell.
https://github.com/audacity/audacity/commit/4251ef8c48e53d26
9fde7d5f0f55ca74bd663a76
mRuler->Refresh();
in void TrackPanel::UpdateVRulerSize() ( in turn called from
UpdateVRuler)
being called too often. That line is responsible for updating the
HORIZONTAL ruler, in the case that the changes in the vertical ruler
changed the width of the vertical ruler (e.g. by causing longer numbers to
be displayed than before). You get that when you zoom in on the vertical
ruler by clicking on it, for example. However it was also being called for
a vertical ruler height change, which should NOT affect the horizontal
ruler.
--James.
In the spectrogram view, live resizing a stereo track lags severely, either
by dragging the middle divider or edge. Once the mouse stops dragging (drag
paused) or button is released (drag ends) it finally gets to draw. If the
track is mono, there is no lag issue.
The reason for lag appears to be a flood of calls to
void TrackPanel::OnTrackListResized(wxCommandEvent & e)
{
Track *t = (Track *) e.GetClientData();
UpdateVRuler(t);
e.Skip();
}
If the call to UpdateVRuler here is removed, the problem
disappears. Why
this is exactly, I can't say. I suspect there is a recursion, e.g.
UpdateVRuler causes a resize event, then ends up back in
UpdateVRuler
again. But it only occurs in spectrogram view for some reason. Another
oddity, while playing back a track, the resize is responsive again.
In any case, since UpdateVRuler is called separately before every refresh
of track anyways, I think this is OK to remove.
------------------------------------------------------------------------------
_______________________________________________
audacity-devel mailing
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
------------------------------------------------------------
------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
------------------------------------------------------------------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
------------------------------------------------------------------------------
_______________________________________________
audacity-devel mailing list
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Loading...