Skip to content

Commit

Permalink
Merge branch 'ht1_2018_fixes' into handout
Browse files Browse the repository at this point in the history
  • Loading branch information
pierremoreau committed Oct 13, 2018
2 parents 54f8681 + 439c454 commit eca0cdf
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 25 deletions.
2 changes: 1 addition & 1 deletion shaders/EDAF80/binormal.vert
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ out VS_OUT {

void main()
{
vs_out.binormal = binormal;
vs_out.binormal = normalize(vec3(vertex_model_to_world * vec4(binormal, 1.0)));

gl_Position = vertex_world_to_clip * vertex_model_to_world * vec4(vertex, 1.0);
}
Expand Down
4 changes: 2 additions & 2 deletions shaders/EDAF80/default.frag
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#version 410

uniform sampler2D diffuse_texture;
uniform int has_textures;
uniform int has_diffuse_texture;

in VS_OUT {
vec2 texcoord;
Expand All @@ -11,7 +11,7 @@ out vec4 frag_color;

void main()
{
if (has_textures != 0)
if (has_diffuse_texture != 0)
frag_color = texture(diffuse_texture, fs_in.texcoord);
else
frag_color = vec4(1.0);
Expand Down
2 changes: 1 addition & 1 deletion shaders/EDAF80/normal.frag
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ out vec4 frag_color;

void main()
{
frag_color = vec4((fs_in.normal + 1.0) / 2.0, 1.0);
frag_color = vec4((normalize(fs_in.normal) + 1.0) / 2.0, 1.0);
}
2 changes: 1 addition & 1 deletion shaders/EDAF80/normal.vert
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ out VS_OUT {

void main()
{
vs_out.normal = vec3(normal_model_to_world * vec4(normal, 0.0));
vs_out.normal = normalize(vec3(normal_model_to_world * vec4(normal, 0.0)));

gl_Position = vertex_world_to_clip * vertex_model_to_world * vec4(vertex, 1.0);
}
Expand Down
2 changes: 1 addition & 1 deletion shaders/EDAF80/tangent.vert
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ out VS_OUT {

void main()
{
vs_out.tangent = normalize(tangent);
vs_out.tangent = normalize(vec3(vertex_model_to_world * vec4(tangent, 1.0)));

gl_Position = vertex_world_to_clip * vertex_model_to_world * vec4(vertex, 1.0);
}
Expand Down
43 changes: 40 additions & 3 deletions src/EDAF80/assignment2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,20 @@ edaf80::Assignment2::run()
if (normal_shader == 0u)
LogError("Failed to load normal shader");

GLuint tangent_shader = 0u;
program_manager.CreateAndRegisterProgram({ { ShaderType::vertex, "EDAF80/tangent.vert" },
{ ShaderType::fragment, "EDAF80/tangent.frag" } },
tangent_shader);
if (tangent_shader == 0u)
LogError("Failed to load tangent shader");

GLuint binormal_shader = 0u;
program_manager.CreateAndRegisterProgram({ { ShaderType::vertex, "EDAF80/binormal.vert" },
{ ShaderType::fragment, "EDAF80/binormal.frag" } },
binormal_shader);
if (binormal_shader == 0u)
LogError("Failed to load binormal shader");

GLuint texcoord_shader = 0u;
program_manager.CreateAndRegisterProgram({ { ShaderType::vertex, "EDAF80/texcoord.vert" },
{ ShaderType::fragment, "EDAF80/texcoord.frag" } },
Expand All @@ -111,6 +125,10 @@ edaf80::Assignment2::run()
// it can always be changed at runtime through the "Scene Controls" window.
bool use_linear = true;

// Set whether to interpolate the position of an object or not; it can
// always be changed at runtime through the "Scene Controls" window.
bool interpolate = true;

auto circle_rings = Node();
circle_rings.set_geometry(shape);
circle_rings.set_program(&fallback_shader, set_uniforms);
Expand Down Expand Up @@ -171,6 +189,12 @@ edaf80::Assignment2::run()
circle_rings.set_program(&normal_shader, set_uniforms);
}
if (inputHandler.GetKeycodeState(GLFW_KEY_4) & JUST_PRESSED) {
circle_rings.set_program(&tangent_shader, set_uniforms);
}
if (inputHandler.GetKeycodeState(GLFW_KEY_5) & JUST_PRESSED) {
circle_rings.set_program(&binormal_shader, set_uniforms);
}
if (inputHandler.GetKeycodeState(GLFW_KEY_6) & JUST_PRESSED) {
circle_rings.set_program(&texcoord_shader, set_uniforms);
}
if (inputHandler.GetKeycodeState(GLFW_KEY_Z) & JUST_PRESSED) {
Expand All @@ -191,8 +215,20 @@ edaf80::Assignment2::run()
circle_rings.rotate_y(0.01f);


//! \todo Interpolate the movement of a shape between various
//! control points
if (interpolate) {
//! \todo Interpolate the movement of a shape between various
//! control points.
if (use_linear) {
//! \todo Compute the interpolated position
//! using the linear interpolation.
}
else {
//! \todo Compute the interpolated position
//! using the Catmull-Rom interpolation;
//! use the `catmull_rom_tension`
//! variable as your tension argument.
}
}


int framebuffer_width, framebuffer_height;
Expand All @@ -206,8 +242,9 @@ edaf80::Assignment2::run()

bool const opened = ImGui::Begin("Scene Controls", nullptr, ImVec2(300, 100), -1.0f, 0);
if (opened) {
ImGui::SliderFloat("Catmull-Rom tension", &catmull_rom_tension, 0.0f, 1.0f);
ImGui::Checkbox("Enable interpolation", &interpolate);
ImGui::Checkbox("Use linear interpolation", &use_linear);
ImGui::SliderFloat("Catmull-Rom tension", &catmull_rom_tension, 0.0f, 1.0f);
}
ImGui::End();

Expand Down
2 changes: 2 additions & 0 deletions src/EDAF80/assignment3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ edaf80::Assignment3::run()
glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
glClear(GL_DEPTH_BUFFER_BIT | GL_COLOR_BUFFER_BIT);

circle_ring.render(mCamera.GetWorldToClipMatrix(), circle_ring.get_transform());

glPolygonMode(GL_FRONT_AND_BACK, GL_FILL);

bool opened = ImGui::Begin("Scene Control", &opened, ImVec2(300, 100), -1.0f, 0);
Expand Down
2 changes: 1 addition & 1 deletion src/EDAF80/parametric_shapes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ parametric_shapes::createSphere(unsigned int const res_theta,
unsigned int const res_phi, float const radius)
{

//! \todo (Optional) Implement this function
//! \todo Implement this function
return bonobo::mesh_data();
}

Expand Down
4 changes: 4 additions & 0 deletions src/EDAN35/assignment2.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// Do not use intrinsic functions, which allows using `constexpr` on GLM
// functions.
#define GLM_FORCE_PURE 1

#include "assignment2.hpp"

#include "config.hpp"
Expand Down
2 changes: 1 addition & 1 deletion src/core/FPSCamera.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class FPSCamera
~FPSCamera();

public:
void Update(double dt, InputHandler &ih);
void Update(double dt, InputHandler &ih, bool ignoreKeyEvents = false, bool ignoreMouseEvents = false);
void SetProjection(T fovy, T aspect, T nnear, T nfar);
void SetFov(T fovy);
T GetFov();
Expand Down
6 changes: 3 additions & 3 deletions src/core/FPSCamera.inl
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ T FPSCamera<T, P>::GetAspect()


template<typename T, glm::precision P>
void FPSCamera<T, P>::Update(double dt, InputHandler &ih)
void FPSCamera<T, P>::Update(double dt, InputHandler &ih, bool ignoreKeyEvents, bool ignoreMouseEvents)
{
glm::tvec2<T, P> newMousePosition = glm::tvec2<T, P>(ih.GetMousePosition().x, ih.GetMousePosition().y);
glm::tvec2<T, P> mouse_diff = newMousePosition - mMousePosition;
mouse_diff.y = -mouse_diff.y;
mMousePosition = newMousePosition;
mouse_diff *= mMouseSensitivity;

if (!ih.IsMouseCapturedByUI() && (ih.GetMouseState(GLFW_MOUSE_BUTTON_LEFT) & PRESSED)) {
if (!ih.IsMouseCapturedByUI() && !ignoreMouseEvents && (ih.GetMouseState(GLFW_MOUSE_BUTTON_LEFT) & PRESSED)) {
mRotation.x -= mouse_diff.x;
mRotation.y += mouse_diff.y;
mWorld.SetRotateX(mRotation.y);
Expand All @@ -65,7 +65,7 @@ void FPSCamera<T, P>::Update(double dt, InputHandler &ih)
T movement = movementModifier * T(dt) * mMovementSpeed;

T move = 0.0f, strafe = 0.0f, levitate = 0.0f;
if (!ih.IsKeyboardCapturedByUI()) {
if (!ih.IsKeyboardCapturedByUI() && !ignoreKeyEvents) {
if ((ih.GetKeycodeState(GLFW_KEY_W) & PRESSED)) move += movement;
if ((ih.GetKeycodeState(GLFW_KEY_S) & PRESSED)) move -= movement;
if ((ih.GetKeycodeState(GLFW_KEY_A) & PRESSED)) strafe -= movement;
Expand Down
4 changes: 4 additions & 0 deletions src/core/WindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ namespace

void FramebufferSizeCallback(GLFWwindow* window, int width, int height)
{
if (width <= 0 || height <= 0)
return;

WindowManager::WindowDatum* const instance = static_cast<WindowManager::WindowDatum*>(glfwGetWindowUserPointer(window));
instance->camera.SetAspect(static_cast<float>(width) / static_cast<float>(height));
}
Expand Down Expand Up @@ -148,6 +151,7 @@ GLFWwindow* WindowManager::CreateWindow(std::string const& title, WindowDatum co
{
#if DEBUG_LEVEL >= 2
glEnable(GL_DEBUG_OUTPUT);
glEnable(GL_DEBUG_OUTPUT_SYNCHRONOUS);
glDebugMessageCallback(utils::opengl::debug::opengl_error_callback, nullptr);
#endif
#if DEBUG_LEVEL == 2
Expand Down
17 changes: 14 additions & 3 deletions src/core/helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,20 @@ bonobo::createTexture(uint32_t width, uint32_t height, GLenum target, GLint inte
glGenTextures(1, &texture);
assert(texture != 0u);
glBindTexture(target, texture);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
glTexImage2D(target, 0, internal_format, static_cast<GLsizei>(width), static_cast<GLsizei>(height), 0, format, type, data);
glTexParameteri(target, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
glTexParameteri(target, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
switch (target) {
case GL_TEXTURE_1D:
glTexImage1D(target, 0, internal_format, static_cast<GLsizei>(width), 0, format, type, data);
break;
case GL_TEXTURE_2D:
glTexImage2D(target, 0, internal_format, static_cast<GLsizei>(width), static_cast<GLsizei>(height), 0, format, type, data);
break;
default:
glDeleteTextures(1, &texture);
LogError("Non-handled texture target: %08x.\n", target);
return 0u;
}
glBindTexture(target, 0u);

return texture;
Expand Down
11 changes: 3 additions & 8 deletions src/core/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,15 @@ Node::render(glm::mat4 const& WVP, glm::mat4 const& world, GLuint program, std::
glUniformMatrix4fv(glGetUniformLocation(program, "normal_model_to_world"), 1, GL_FALSE, glm::value_ptr(normal_model_to_world));
glUniformMatrix4fv(glGetUniformLocation(program, "vertex_world_to_clip"), 1, GL_FALSE, glm::value_ptr(WVP));

glUniform1i(glGetUniformLocation(program, "has_textures"), !_textures.empty());
bool has_diffuse_texture = false, has_opacity_texture = false;
for (size_t i = 0u; i < _textures.size(); ++i) {
auto const texture = _textures[i];
glActiveTexture(GL_TEXTURE0 + static_cast<GLenum>(i));
glBindTexture(std::get<2>(texture), std::get<1>(texture));
glUniform1i(glGetUniformLocation(program, std::get<0>(texture).c_str()), static_cast<GLint>(i));
if (std::get<0>(texture) == "diffuse_texture")
has_diffuse_texture = true;
else if (std::get<0>(texture) == "opacity_texture")
has_opacity_texture = true;

std::string texture_presence_var_name = "has_" + std::get<0>(texture);
glUniform1i(glGetUniformLocation(program, texture_presence_var_name.c_str()), 1);
}
glUniform1i(glGetUniformLocation(program, "has_diffuse_texture"), has_diffuse_texture);
glUniform1i(glGetUniformLocation(program, "has_opacity_texture"), has_opacity_texture);

glBindVertexArray(_vao);
if (_has_indices)
Expand Down

1 comment on commit eca0cdf

@4onen
Copy link

@4onen 4onen commented on eca0cdf Oct 13, 2018

Choose a reason for hiding this comment

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

The change to src/core/node.cpp breaks more things than it fixes, as uniforms are not reset between render calls. For example, in one assignment I render a thing with a diffuse_texture sampler2D. Immediately after, I render another object using the same shader program that is without that texture. Before, because has_diffuse_texture was set per-object, it was false on the second render call and my object rendered without the diffuse texture as expected. After this change, has_diffuse_texture is not reset to false by OpenGL on my platform, so the textureless object erroneously shows up with a texture (pulled from whatever was active in GL_TEXTURE0.)

Personally, I'd call this a bug and not a feature. But it is your code; teach as you will. Admittedly, this behaviour is neat and could potentially save cycles in a careful setup.

Please sign in to comment.