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

Lesson 2 first triangle function is incorrect and image in wiki doesn't match actual output #102

Open
phinjensen opened this issue Jun 3, 2022 · 2 comments

Comments

@phinjensen
Copy link

I've been working through the code and ran into this issue trying to port it to Rust. Rendering a couple of the example triangles using the initial line sweeping approach resulted in odd flat edges (scaled 4x with problem highlighted):

bad_edges

I checked out the code at commit 024ad46 (the linked commit), replaced its version of triangle() with the first one given, and found that it was also giving the weird edges, so it isn't just a problem with my code:

void triangle(Vec2i t0, Vec2i t1, Vec2i t2, TGAImage &image, TGAColor color) {                                                                                                                                                                 
-    if (t0.y==t1.y && t0.y==t2.y) return; // i dont care about degenerate triangles                                                                                                                                                            
-    if (t0.y>t1.y) std::swap(t0, t1);                                                                                                                                                                                                          
-    if (t0.y>t2.y) std::swap(t0, t2);                                                                                                                                                                                                          
-    if (t1.y>t2.y) std::swap(t1, t2);                                                                                                                                                                                                          
-    int total_height = t2.y-t0.y;                                                                                                                                                                                                              
-    for (int i=0; i<total_height; i++) {                                                                                                                                                                                                       
-        bool second_half = i>t1.y-t0.y || t1.y==t0.y;                                                                                                                                                                                          
-        int segment_height = second_half ? t2.y-t1.y : t1.y-t0.y;                                                                                                                                                                              
-        float alpha = (float)i/total_height;                                                                                                                                                                                                   
-        float beta  = (float)(i-(second_half ? t1.y-t0.y : 0))/segment_height; // be careful: with above conditions no division by zero here                                                                                                   
-        Vec2i A =               t0 + (t2-t0)*alpha;                                                                                                                                                                                            
-        Vec2i B = second_half ? t1 + (t2-t1)*beta : t0 + (t1-t0)*beta;                                                                                                                                                                         
-        if (A.x>B.x) std::swap(A, B);                                                                                                                                                                                                          
-        for (int j=A.x; j<=B.x; j++) {                                                                                                                                                                                                         
-            image.set(j, t0.y+i, color); // attention, due to int casts t0.y+i != A.y                                                                                                                                                          
-        }                                                                                                                                                                                                                                      
-    }                                                                                                                                                     
+    // sort the vertices, t0, t1, t2 lower−to−upper (bubblesort yay!)                                                                                                                                                                          
+    if (t0.y>t1.y) std::swap(t0, t1);                                                                                                                                                                                                          
+    if (t0.y>t2.y) std::swap(t0, t2);                                                                                                                                                                                                          
+    if (t1.y>t2.y) std::swap(t1, t2);                                                                                                                                                                                                          
+    int total_height = t2.y-t0.y;                                                                                                                                                                                                              
+    for (int y=t0.y; y<=t1.y; y++) {                                                                                                                                                                                                           
+        int segment_height = t1.y-t0.y+1;                                                                                                                                                                                                      
+        float alpha = (float)(y-t0.y)/total_height;                                                                                                                                                                                            
+        float beta  = (float)(y-t0.y)/segment_height; // be careful with divisions by zero                                                                                                                                                     
+        Vec2i A = t0 + (t2-t0)*alpha;                                                                                                                                                                                                          
+        Vec2i B = t0 + (t1-t0)*beta;                                                                                                                                                                                                           
+        if (A.x>B.x) std::swap(A, B);                                                                                                                                                                                                          
+        for (int j=A.x; j<=B.x; j++) {                                                                                                                                                                                                         
+            image.set(j, y, color); // attention, due to int casts t0.y+i != A.y                                                                                                                                                               
+        }                                                                                                                                                                                                                                      
+    }                                                                                                                                                                                                                                          
+    for (int y=t1.y; y<=t2.y; y++) { 
+        int segment_height =  t2.y-t1.y+1;                 
+        float alpha = (float)(y-t0.y)/total_height; 
+        float beta  = (float)(y-t1.y)/segment_height; // be careful with divisions by zero 
+        Vec2i A = t0 + (t2-t0)*alpha;                  
+        Vec2i B = t1 + (t2-t1)*beta; 
+        if (A.x>B.x) std::swap(A, B); 
+        for (int j=A.x; j<=B.x; j++) { 
+            image.set(j, y, color); // attention, due to int casts t0.y+i != A.y                                       
+        }    
+    }

But comparing it with that working version, I also found the issue is just in the calculation of segment_height, which shouldn't have the +1 in both cases. Removing that fixed the line drawing. Can we get the tutorial code or images updated to match each other?

@DouYuexi

This comment was marked as spam.

@XiaofanLinUS
Copy link

I am pretty sure that +1 is used to avoid div by 0. I was confused by that too when I was doing it. I think it's a hack. The triangle may look a little bit odd in a large image. By when you scale up the scene, the small error won't matter I guess.

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

3 participants