Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Platform dependent fixes #2

Closed
Xin-888 opened this issue Feb 2, 2021 · 3 comments
Closed

Platform dependent fixes #2

Xin-888 opened this issue Feb 2, 2021 · 3 comments

Comments

@Xin-888
Copy link

Xin-888 commented Feb 2, 2021

Hello, here are the fixes I needed to properly generate the displacement textures on windows (I compiled with Code::Blocks MinGW x64 and tested it with an older graphics card):

  • In

    glm::dvec3 d0, d1, d2;
    you need to make sure to value initialize the dvec3s, there is no guarantee that they will be value initialized.

  • In

    glReadnPixels(0, 0, width, width, GL_RGB, GL_UNSIGNED_SHORT, data.size() * sizeof(Pixel), data.data());
    you could use the older version "glReadPixels(0, 0, width, width, GL_RGB, GL_UNSIGNED_SHORT, data.data());" to avoid crashes with older cards that don't support the latest versions of OpenGL.

  • Also around

    you have to make sure to call "glViewport(0, 0, width, width);", since there is no guarantee that glfwCreateWindow() will have the supplied resolution, so the mapping to screen coordinates can end up buggy once the framebuffer is set up.

  • I personally didn't experience the following issue, but it's a good practice to complete the framebuffer. Something like:

      GLuint renderBuffer;
      glGenRenderbuffers(1, &renderBuffer);
      glBindRenderbuffer(GL_RENDERBUFFER, renderBuffer);
      glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, width, width);
      glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_STENCIL_ATTACHMENT, GL_RENDERBUFFER, renderBuffer);
      if (glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE)
          throw std::runtime_error("framebuffer is not complete.");
      glDisable(GL_DEPTH_TEST);
    
  • While we are at it, a useful qol change would be to keep track of the most positive and most negative pixel values to help detect scaling issues. Something like (in diff.cc):

      ...
      
      auto maxp = std::max(displ2.x, std::max(displ2.y, displ2.z));
      maxDispl_positive = std::max(maxDispl_positive, maxp);
    
      auto maxn = std::min(displ2.x, std::min(displ2.y, displ2.z));
      maxDispl_negative = std::min(maxDispl_negative, maxn);
              
      ...
              
      std::cerr << "Most positive pixel component value: " << (maxDispl_positive * scale + 0.5) * 255.0 << "\n";
      std::cerr << "Most negative pixel component value: " << (maxDispl_negative * scale + 0.5) * 255.0 << "\n";
    
edolstra added a commit that referenced this issue Feb 7, 2021
In glm 0.9.8 (Nixpkgs 20.09) vecs are still zero-initialized by
default, but in 0.9.9 they're not. So restore the old behaviour.

Issue #2.
edolstra added a commit that referenced this issue Feb 7, 2021
edolstra added a commit that referenced this issue Feb 7, 2021
@edolstra
Copy link
Owner

edolstra commented Feb 7, 2021

Thanks @Xin-888, I've applied your suggestions!

@edolstra edolstra closed this as completed Feb 7, 2021
@Xin-888
Copy link
Author

Xin-888 commented Feb 7, 2021

Thanks for the quick fixes @edolstra . By the way, I think there is a typo in

if (std::max(maxPos, abs(maxPos)) > 1e-6) {
, you meant "abs(maxNeg)" not "abs(maxPos)".

Thanks again.

@edolstra
Copy link
Owner

edolstra commented Feb 8, 2021

Thanks, fixed!

edolstra added a commit that referenced this issue Aug 26, 2021
In glm 0.9.8 (Nixpkgs 20.09) vecs are still zero-initialized by
default, but in 0.9.9 they're not. So restore the old behaviour.

Issue #2.
edolstra added a commit that referenced this issue Aug 26, 2021
edolstra added a commit that referenced this issue Aug 26, 2021
edolstra added a commit that referenced this issue Aug 26, 2021
In glm 0.9.8 (Nixpkgs 20.09) vecs are still zero-initialized by
default, but in 0.9.9 they're not. So restore the old behaviour.

Issue #2.
edolstra added a commit that referenced this issue Aug 26, 2021
edolstra added a commit that referenced this issue Aug 26, 2021
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

No branches or pull requests

2 participants