Skip to content

Remove instrument track window caching to fix related bugs - #5808

Merged
PhysSong merged 3 commits into
LMMS:masterfrom
PhysSong:oneinstwin
Dec 1, 2020
Merged

Remove instrument track window caching to fix related bugs#5808
PhysSong merged 3 commits into
LMMS:masterfrom
PhysSong:oneinstwin

Conversation

@PhysSong

Copy link
Copy Markdown
Member

Fixes #3623, fixes #5743.

Also makes the behavior better when toggling one instrument track mode.
delete m_instrumentView;

if( gui->mainWindow()->workspace() )
if (parentWidget())

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to prevent crashes on closing.

@LmmsBot

LmmsBot commented Nov 28, 2020

Copy link
Copy Markdown
🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://1197-94782492-gh.circle-artifacts.com/0/lmms-1.2.3-780%2Bg85a70d1-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/PhysSong/lmms/1197?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://1200-94782492-gh.circle-artifacts.com/0/lmms-1.2.3-780%2Bg85a70d1a4-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/PhysSong/lmms/1200?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://1196-94782492-gh.circle-artifacts.com/0/lmms-1.2.3-780%2Bg85a70d1a4-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/PhysSong/lmms/1196?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/16rri9l8h6h8mts0/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36570105"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/xycs9mpf3f5rx518/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/36570105"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://1199-94782492-gh.circle-artifacts.com/0/lmms-1.2.3-780%2Bg85a70d1a4-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/PhysSong/lmms/1199?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "85a70d1a407eb6337f3a7291f5f17034527c36cc"}

@JohannesLorenz

Copy link
Copy Markdown
Contributor

Give me some time and I'll review this PR.

@JohannesLorenz
JohannesLorenz requested review from JohannesLorenz and Veratil and removed request for Veratil November 28, 2020 20:20
@PhysSong PhysSong mentioned this pull request Nov 29, 2020

@JohannesLorenz JohannesLorenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only 3 very minor comments.

I also tested everything except the Carla issue #5743 .

LGTM.

Comment thread include/InstrumentTrack.h
Comment thread src/tracks/InstrumentTrack.cpp Outdated
InstrumentTrackView::~InstrumentTrackView()
{
freeInstrumentTrackWindow();
delete m_window;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using unique_ptr instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's out of scope, but it's doable.

@PhysSong
PhysSong merged commit d73ede5 into LMMS:master Dec 1, 2020
@PhysSong
PhysSong deleted the oneinstwin branch December 1, 2020 02:04
IanCaio pushed a commit to IanCaio/lmms that referenced this pull request Mar 28, 2021
devnexen pushed a commit to devnexen/lmms that referenced this pull request Apr 10, 2021
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
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.

Crash when Carla Instrument is removed Bugs with toggling One Instrument Track Window Mode

3 participants