Skip to content

Replace boost::filter_iterator with explicit iterator implementation#2312

Merged
pixar-oss merged 1 commit intoPixarAnimationStudios:devfrom
nvmkuruc:sdffilteriterator
Jul 27, 2023
Merged

Replace boost::filter_iterator with explicit iterator implementation#2312
pixar-oss merged 1 commit intoPixarAnimationStudios:devfrom
nvmkuruc:sdffilteriterator

Conversation

@nvmkuruc
Copy link
Copy Markdown
Collaborator

Description of Change(s)

Converts the _Predicate private class into an explicit _FilterIterator bidirectional iterator implementation.

Fixes Issue(s)

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

@tallytalwar
Copy link
Copy Markdown
Contributor

Filed as internal issue #USD-8056

Comment thread pxr/usd/sdf/childrenView.h Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm pretty sure you have this inverted. According to the boost filter_iterator documentation, the iterator retains the element if the predicate returns true and skips the element if the predicate is false. Here, you're skipping when the predicate is true.

Copy link
Copy Markdown
Collaborator Author

@nvmkuruc nvmkuruc Jul 11, 2023

Choose a reason for hiding this comment

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

That was it. All tests are now passing.

Comment thread pxr/usd/sdf/childrenView.h Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These shouldFilter and filter functions aren't here for API conformance right? If I'm right, they should follow our coding conventions for private methods, i.e. capitalized and starting with an underscore

Comment thread pxr/usd/sdf/childrenView.h Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The body of all three of these should be on a new line like you did for all the rest of the functions.

@nvmkuruc nvmkuruc force-pushed the sdffilteriterator branch from 1997562 to bce13c5 Compare July 11, 2023 04:04
@pixar-oss pixar-oss merged commit 0104473 into PixarAnimationStudios:dev Jul 27, 2023
@nvmkuruc nvmkuruc deleted the sdffilteriterator branch December 29, 2023 03:12
musicinmybrain pushed a commit to musicinmybrain/OpenUSD that referenced this pull request Apr 6, 2026
…ationStudios#2312)

The B44 and B44A decoder and encoder use channel width (`nx`) and
height (`ny`) in row pointer math. `nx` and `ny` are `int`; the
scratch buffer is correctly sized with `(uint64_t)ny * (uint64_t)nx *
bytes_per_element`, but row bases were computed as:

```
  row0 = (uint16_t*)scratch;
  row0 += y * nx;   // int * int -> signed overflow when y*nx > INT_MAX
```

For large `nx` (e.g. 268435456), `y*nx` overflows, so `row0`/`row1`/`row2`/`row3`
point before the scratch buffer.

Fix: compute the row offset in `uint64_t` before pointer arithmetic in both
`uncompress_b44_impl` (decoder) and `compress_b44_impl` (encoder).

Analysis and solution with the help of Curor / Claude Opus 4.5

Signed-off-by: Cary Phillips <cary@ilm.com>
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.

4 participants