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

Build with cmake & bugfixes #13

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

Conversation

jbehley
Copy link

@jbehley jbehley commented Mar 27, 2018

Hi Joey,

since I was interested how a modern rendering framework, I stumbled over your engine. I really enjoyed your learnopengl tutorials and therefore I directly cloned your repository. I basically learned everything about modern OpenGL from these tutorials! I'm really looking forward to your learnvulkan project.

I was really interested how it really looks on my Ubunutu and therefore I had to do some changes to get it working. (At least now it compiles with Ubuntu 16.04 and CMake on my system.)

Since gcc seems to be more pedantic than the MS compiler, I had to do quite some modifications and therefore I understand if you don't want to integrate these changes in your repository. However, you might want to have a look at some pointer initializations in your code, since they caused some segmentation faults under linux.

Just wanted to let you know that your renderer works also fine under Ubuntu! 👍

Update: I also fixed the shader errors that were reported by an issue. I use also a Nvidia graphics card.

jbehley and others added 27 commits March 27, 2018 10:35
…ow it runs, ... but still a lot of pointers uninitalized.
@JoeyDeVries
Copy link
Owner

Hey @jbehley, thanks for the great work! I took a quick look today at your changes and there were only a few smaller parts that I would've done/named differently, but these are minor compared to the advantage of having it all work on Linux (and with a proper CMakeLists file :)). I'll try and run a test later this week to see if everything works on my end as well given these changes (and fix some small issues if they become apparent) and if so, I'll merge them in, thank you!

@jbehley
Copy link
Author

jbehley commented Mar 28, 2018

Hi @JoeyDeVries, I didn't manage to figure out how to really solve the problem with the capture.vs and capture.fs shaders (see also discussion in #6).

The naive solution with just using the UBO in the vertex shader did produce very dark lighting results (due to the issue with the wrong projection & view matrix mentioned in issue #6).

However, the current solution does improve this behavior visually, but does not produce correct results with the lighting on the torus. So here a deeper understanding of the baking would be needed to correct the results. The debug probes also seem to produce the wrong results. Anyway, here some additional work is needed.

Side note: I also noticed the "black rectangle" that pops up when the camera is moving.

@gameengineer
Copy link

Hi jbehley,
I completely admit I am no GIT expert so would you explain what are the git commands to get your pull #13? I want to build and run with VS2015 although I have them all anyway but I think GLFW binaries are VS2015. I use NVidia cards.

@jbehley
Copy link
Author

jbehley commented Apr 17, 2018

Hi @gameengineer,

the simplest thing is to just use my fork, i.e.,
git clone --recurse-submodules https://github.com/jbehley/Cell.git
and then compile it with cmake. (See also section Build in the README.md of the fork.)

However, I think the solution file should still work. Would be nice if you could check this and give some feedback. (I only tested the thing with Ubuntu 16.04. Thus, I don't know whether the Microsoft compiler now complains after the numerous changes.)

@gameengineer
Copy link

gameengineer commented Apr 17, 2018 via email

@ghost
Copy link

ghost commented Dec 16, 2018

Hi @JoeyDeVries, how it is working with this pull request, I would like to test this engine, but I am on Ubuntu and CMake it will be a great addition to the project.

@Crisspl
Copy link

Crisspl commented Dec 16, 2018

@iamDreamyComet did you read the post by the PR author at all?

@ghost
Copy link

ghost commented Dec 16, 2018

@iamDreamyComet did you read the post by the PR author at all?

Yes and I am waiting for the PR to be in the master branch.

@Crisspl
Copy link

Crisspl commented Dec 16, 2018

Well, you can clone from @jbehley. Author claims that it works on config similar to yours, so...

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