leozide / leocad

A CAD application for creating virtual LEGO models
https://www.leocad.org
2.47k stars 208 forks source link

PATCH: Enable Back Face Culling (bfc) #639

Open rsbx opened 3 years ago

rsbx commented 3 years ago

EDIT: I've been using the patch for a while without issues so I think it's ready. See later comments for the latest version.

Original comment follows: WARNING: The attached patch is incomplete in that LeoCAD does not track, or even parse, BFC CLIP and BFC NOCLIP meta commands and this patch does not change that. It's not an issue for most parts in the current LDraw parts library but a quick grep shows that there are still some with a BFC NOCLIP meta command and 3 with a BFC NOCERTIFY meta command.

rsbx commented 3 years ago

Attached is a patch that enables working Back Face Culling in LeoCAD.

bfc.patch.txt

I didn't (and still don't) really know how all of the mesh related code works, but it seems to work.

Limitations:

Fun extra: The studs texture on the base grid were actually on the bottom of the base grid plane so I inverted the base grid plane and enabled BFC on it. Now the studs texture is only visible from above.

rsbx commented 3 years ago

Attached is an updated patch to fix a normal inversion issue in the previous patch. Normals should now be the same as LeoCAD without this BFC patch.

bfc.patch.txt

rsbx commented 3 years ago

@leozide Bump

rsbx commented 3 years ago

Updated patch: bfc.patch.txt

leozide commented 3 years ago

I've looked at this today:

rsbx commented 3 years ago
rsbx commented 3 years ago

Updated patch:

bfc.patch.txt

leozide commented 3 years ago

I've checked in a change to print draw times (see lcViewWidget::paintGL), do you mind turning it on and running with and without this patch a couple of times?

On both my computers it was about 5% slower on the CPU and made no difference on the GPU as I expected (I tried BFC in the past and had the same results so I never looked into it again). My desktop was GPU bound so it didn't matter much but my laptop is CPU bound so it's slightly slower.

I tested with the tower bridge model from the OMR.

rsbx commented 3 years ago

I'll be a couple of hours before I can do any repeatable testing but a quick test of commit c613ff6, "Added simple draw timer", with the timer enabled is continously consuming both CPUs in the VM even with no model loaded. It also segfaults when I quit LeoCAD.

I run LeoCAD from an xterm so a simple printf() without the QT machinery might be sufficient and/or better for this profiling task.

rsbx commented 3 years ago

To be clear, the rendering time improvement I see with the BFC patch is when I rotating an already loaded model via the mouse. It does not appear to affect (perceptively) the time to load a model.

rsbx commented 3 years ago

Here are the the master and master+BFC timings for 3 different models on both the VM (llvm based 3D) and the host (Intel integrated graphics based 3D):

vm_timings.txt host_timings.txt

The last several updates in each test are due to just hitting the H (home view) shortcut.

I may not have been "twisting" the c11-color model fast enough during the host master+bfc timing run.

I used a simpler timing patch that did not try to consume all my CPUs:

`#ifndef LC_PROFILE_DRAW mView->OnDraw();

else

    struct timespec ts0, ts1;
    unsigned long elapsed;

    clock_gettime(CLOCK_MONOTONIC, &ts0);
    mView->OnDraw();
    clock_gettime(CLOCK_MONOTONIC, &ts1);

    elapsed = (ts1.tv_sec - ts0.tv_sec) * 1000000000 + ts1.tv_nsec - ts0.tv_nsec;
    printf("%lu\n", elapsed); 

endif

`

rsbx commented 3 years ago

Playing with commit b6acab71 and the on-screen timer display, the GPU values, of the 3 large models I tested, with the BFC patch is about 75% of the GPU values without the patch. This is true for both the VM and the host.

The CPU value of the largest model with the BFC patch is about 75% of the CPU value without the patch on the VM; on the HOST it's about 93%. The other models appeared similar but I wasn't watching the CPU value too closely.

rsbx commented 3 years ago

Rebased onto commit 1a64a787:

Back-Face-Culling.patch.txt

rsbx commented 3 years ago

Rebased onto commit 613bc91c:

0001-Back-Face-Culling.patch.txt

rsbx commented 3 years ago

Rebased onto commit 35ba24fb:

0001-Back-Face-Culling.patch.txt

rsbx commented 3 years ago

This updated patch, still relative to 35ba24f, now also enables back face culling for synthesized parts (lc_synth.cpp):

0001-Back-Face-Culling.patch.txt

rsbx commented 2 years ago

Rebased onto f24317e

0001-Back-Face-Culling.patch.txt