taiya / dgp

Digital Geometry Processing - Fall 2016 - University of Victoria
38 stars 11 forks source link

Code base issue in Laplacian.h #21

Closed xuzheng0927 closed 7 years ago

xuzheng0927 commented 7 years ago

@nlguillemot I found some issues in Laplacian.h, starting from line 87. Please kindly see the comments:

            cotanAlpha = 1.0f / std::tan(alpha);
            cotanBeta = 1.0f / std::tan(beta);
            // Should get rid of minus values

            omegaList.push_back(Triplet(v_i.idx(), v_j.idx(), -1)); // Should push in (cotanAlpha+cotanBeta) ?
            cotanSum += cotanAlpha + cotanBeta;

            area += (1 / 6.0f) * (d_ij.cross(d_ia)).norm();
            degree++;
        }

        omegaList.push_back(Triplet(v_i.idx(), v_i.idx(),
                                    (Scalar)degree)); // Should push in -cotanSum?

        areaList.push_back(Triplet(v_i.idx(), v_i.idx(),
                                   1.0f / (2.0f * area))); 
    }

    L_omega.setFromTriplets(omegaList.begin(), omegaList.end());
    Area.setFromTriplets(areaList.begin(), areaList.end());

    return Area * L_omega;
xuzheng0927 commented 7 years ago

The final L_omega(i, j) is equal to 2/A_i*(cot_a_ij+cot_b_ij) when i!=j, right? I fixed the code by myself, but why is the deformation output still not correct? I used the graph laplacian and the output was ok. So weird.

taiya commented 7 years ago

I would suggest you to start working on part 2 and let the TA the time to look at the code. On regular meshes the graph laplacian will be enough to get you up and running

xuzheng0927 commented 7 years ago

Sounds good. I'll start working on part 2 tomorrow.

On Tue, Nov 15, 2016 at 10:30 PM, Andrea notifications@github.com wrote:

I would suggest you to start working on part 2 and let the TA the time to look at the code. On regular meshes the graph laplacian will be enough to get you up and running

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ataiya/dgp/issues/21#issuecomment-260865469, or mute the thread https://github.com/notifications/unsubscribe-auth/AI3gFB1bJDY5h57KIyAcSyuIeiyxEY02ks5q-qMZgaJpZM4KzYwq .

nlguillemot commented 7 years ago

I've integrated your changes. I guess it must have been a copy paste error from the graph laplacian implementation.