Modify list writer to use the smallest counter type that will fit.#59
Open
gilgamec wants to merge 1 commit into
Open
Modify list writer to use the smallest counter type that will fit.#59gilgamec wants to merge 1 commit into
gilgamec wants to merge 1 commit into
Conversation
- Only unsigned integral types allowed by PLY (uint8/16/32_t) are allowed; all other instantiations cause an exception in the constructor. Signed types used in a PLY file are interpreted as their unsigned equivalents. - Added addSizedListProperty method to Element to choose which instantiation to use based on the number of elements in the provided lists.
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
1 participant
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.
Let's try this again (as somehow #57 was closed during my mucking about). Fixes #56.
The modification adds a size-type template parameter to
TypedListProperty. It only allows appropriate unsigned integral sizes (anything else throws from the constructor). (Since the code was already implicitly changing signed integers to unsigned, I figure this cements that.) The most significant changes outsideTypedListPropertyitself are to accessors inElement(e.g.getListPropertyType,getDataFromListPropertyRecursive) to triple the number ofdynamic_casts we have to do in order to find the correct type forTypedListProperty.The existing
ElementmethodaddListPropertyis joined byaddSizedListProperty, which counts the sizes of the input vectors to find the longest, then creates aTypedListPropertywith a size-type which is the smallest that can store the count. In this way, lists of fewer than 256 elements are still saved with a uchar, but longer lists can still be saved.I also added some tests, though I tested only int and float list elements rather than the entire panoply of list type options.