Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Add support for rendering geometry range #59

Merged
merged 1 commit into from
Jan 24, 2019
Merged

Conversation

MortimerGoro
Copy link
Contributor

No description provided.

@MortimerGoro MortimerGoro force-pushed the render_range branch 2 times, most recently from 2e15669 to 7612051 Compare January 24, 2019 19:23
VRB_WARN("Invalid geometry range (%u-%u). Max geometry length %d", m.rangeStart, m.rangeLength + m.rangeLength, maxLength);
} else {
VRB_GL_CHECK(glDrawElements(GL_TRIANGLES, m.rangeLength, GL_UNSIGNED_SHORT, (void*)(m.rangeStart * sizeof(GLushort))));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want to nit pick (but I will 😅). I think I would prefer it be structure one of these two ways:

    if (m.rangeLength == 0) {
      VRB_GL_CHECK(glDrawElements(GL_TRIANGLES, maxLength, GL_UNSIGNED_SHORT, 0));
    } else if ((m.rangeStart + m.rangeLength) <= maxLength) {
      VRB_GL_CHECK(glDrawElements(GL_TRIANGLES, m.rangeLength, GL_UNSIGNED_SHORT, (void*)(m.rangeStart * sizeof(GLushort))));
    } else {
      VRB_WARN("Invalid geometry range (%u-%u). Max geometry length %d", m.rangeStart, m.rangeLength + m.rangeLength, maxLength);
    }

or

    if ((m.rangeStart + m.rangeLength) <= maxLength) {
      const uint32_t length = m.rangeLength ? m.rangeLength : maxLength;
      VRB_GL_CHECK(glDrawElements(GL_TRIANGLES, length, GL_UNSIGNED_SHORT, (void*)(m.rangeStart * sizeof(GLushort))));
    } else {
      VRB_WARN("Invalid geometry range (%u-%u). Max geometry length %d", m.rangeStart, m.rangeLength + m.rangeLength, maxLength);
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

np, PR updated!

@bluemarvin bluemarvin merged commit a4d9785 into master Jan 24, 2019
@bluemarvin bluemarvin deleted the render_range branch January 25, 2019 23:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants