Edit: Fixed toBitSet() ignoring its argument and returning every track in the Edit - #403
Merged
Conversation
…k in the Edit toBitSet() used its argument only to reach the Edit and then looped over getAllTracks(), so the returned bitset never depended on what was asked for. Callers passing a subset - Renderer::Parameters::tracksToDo and Renderer::measureStatistics() via test_utilities::expectPeak - silently addressed the whole Edit. Iterating `tracks` instead makes the result match the documented behaviour, so the private getTracksMask() copies in tracktion_EditNodeBuilder.test.cpp and tracktion_Plugins.test.cpp can go back to calling the shared helper. Added tracktion_EditUtilities.test.cpp covering subsets, the empty array, tracks from another Edit, agreement with Track::getIndexInEditTrackList() and the toTrackArray() round-trip. Fixes #399
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #399
Problem
toBitSet (const juce::Array<Track*>&)(modules/tracktion_engine/model/edit/tracktion_EditUtilities.cpp:250) used its argument only to reach theEdit, then looped overgetAllTracks()instead oftracks:The result therefore never depended on what was asked for - every track in the Edit got a bit. The mask is live:
Renderer::Parameters::tracksToDobecomesCreateNodeParams::allowedTracks(Renderer.cpp:38), so a caller asking for a subset silently rendered or measured the whole Edit.It stayed hidden in-engine because
Renderer::renderToFile()passesgetAllTracks (edit)(for which "every track" is the right answer), and the one in-engine call passing a subset (tracktion_NodeRendering.test.cpp:203) is on an Edit with a single audio track. The engine's own tests had already worked around it:tracktion_EditNodeBuilder.test.cppandtracktion_Plugins.test.cppeach carried a privategetTracksMask()built fromTrack::getIndexInEditTrackList()rather than calling the shared helper.Plan
tracksrather thanallTracksintoBitSet().allTracks.indexOf()still supplies the index, so the bit numbering is unchanged and tracks from another Edit are skipped.tracktion_EditUtilities.hto state what the bits mean, where the Edit comes from, and that foreign tracks are ignored - the vague wording is part of why the bug survived.tracktion_EditUtilities.test.cpp(newENGINE_UNIT_TESTS_EDIT_UTILITIESflag intracktion_TestConfig.h, included fromtracktion_engine_model_1.cpp) covering: a single track, an arbitrary subset, all tracks, an empty array, tracks belonging to another Edit, agreement between the bit index andTrack::getIndexInEditTrackList(), and atoTrackArray()round-trip.getTracksMask()copies intracktion_EditNodeBuilder.test.cppandtracktion_Plugins.test.cppand calltoBitSet()instead - those helpers exist only to route around this bug, and keeping them would leave the fix untested by the suites that actually render subsets.BREAKING-CHANGES.md: host code that passed a subset now gets a subset, which changes what renders.Verification
TestRunnerDebug builds clean, and the suites touching the changed helpers pass:tracktion_EditUtilities.test.cpp,tracktion_EditNodeBuilder.test.cpp,tracktion_Plugins.test.cppandtracktion_NodeRendering.test.cpp- 12 test cases, 543 assertions, 0 failures.toBitSet()into a bitset:TransportControl.cpp:969,AudioTrack.cpp:1094andRenderOptions::getTrackIndexes()all build their masks directly, andRenderer::renderToFile()passes every track, so its result is unchanged.Note on behaviour
test_utilities::expectPeak()intracktion_TestUtilities.hhas no in-engine callers but is part of the public test utilities - host tests using it against a subset of tracks will now measure that subset, which is the documented intent but may shift existing expected values.