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

Batch vertices for large shapes. #170

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

Conversation

Kevinpgalligan
Copy link
Contributor

Fix for this bug: #160

The draw buffer isn't large enough for shapes that consist of more than about 6000 vertices, so we need to batch the draw calls. Different batching behaviour is required for different primitive types, e.g. for triangle strips, in each batch you have to include the last 2 vertices from the previous batch so that the strip is continuous. I've only covered the :triangles and :triangle-strip primitive types.

Testing

This triggers the batching for :triangles. It's basically the example case that Gleefre gave with the bug report, and crashes without batching.

(defsketch huge-polygon ()
    (with-pen (make-pen :fill +white+ :stroke +black+ :weight 2)
      (apply #'polygon (loop repeat 300 collect (random 400))))
    (stop-loop))

Here's how the batching logic works for :triangles. batch-vertices returns a list of batches: each batch consists of the number of vertices in that batch, a pointer to the start of that batch in the list of all vertices, and a boolean indicating whether it's the last batch. I took this approach, rather than returning new lists of vertices, to avoid excessive cons-ing.

(let ((*buffer-size* (* 5 *bytes-per-vertex*)))
          (with-environment (make-env)
            (batch-vertices '(1 2 3 4 5 6 7 8 9) :triangles)))
((3 (1 2 3 4 5 6 7 8 9) NIL) (3 (4 5 6 7 8 9) NIL) (3 (7 8 9) T))
(let ((*buffer-size* (* 6 *bytes-per-vertex*)))
          (with-environment (make-env)
            (batch-vertices '(1 2 3 4 5 6 7 8 9) :triangles)))
((6 (1 2 3 4 5 6 7 8 9) NIL) (3 (7 8 9) T))

This triggers the :triangle-strip batching.

(defsketch huge-polyline ()
    (with-pen (make-pen :stroke +white+ :weight 2)
      (apply #'polyline (loop repeat 10000 collect (random 400))))
    (stop-loop))

And here's the corresponding batching logic. Recall that each batch must contain the last 2 vertices from the previous batch.

SKETCH> (let ((*buffer-size* (* 5 *bytes-per-vertex*)))
          (with-environment (make-env)
            (batch-vertices '(1 2 3 4 5) :triangle-strip)))
((5 (1 2 3 4 5) T))
SKETCH> (let ((*buffer-size* (* 5 *bytes-per-vertex*)))
          (with-environment (make-env)
            (batch-vertices '(1 2 3 4 5 6) :triangle-strip)))
((5 (1 2 3 4 5 6) NIL) (3 (4 5 6) T))
SKETCH> (let ((*buffer-size* (* 5 *bytes-per-vertex*)))
          (with-environment (make-env)
            (batch-vertices '(1 2 3 4 5 6 7 8 9 10) :triangle-strip)))
((5 (1 2 3 4 5 6 7 8 9 10) NIL) (5 (4 5 6 7 8 9 10) NIL) (4 (7 8 9 10) T))

One thing that's not addressed: I think the texture might be discontinuous on a polygon whose constituent triangles are batched. Perhaps that could be fixed by calculating the bounding box once for all the vertices, rather than once per batch? I'm not sure how textures are mapped onto triangles, however, so I'm not sure.

@Kevinpgalligan
Copy link
Contributor Author

Note: besides the handling of textures, I also need to go back over the OpenGL stuff to make sure the new code is handling it correctly. It's difficult to verify that all the batches are being drawn when the polygon is just a big mess of random points, might be worth trying to create a big polygon that can be visually confirmed to be drawn correctly.

@Kevinpgalligan
Copy link
Contributor Author

Confirmed that there are discontinuities in the texture when there's batching.

Example:

(defsketch discontinuity
            ((img (load-resource "/path/to/cat.jpeg"))
             (y-axis :up)
             (num-points 1000))
          (let ((pts
                  (loop for i below num-points
                        for angle = (* 2 pi (/ i num-points))
                        collect (+ (/ width 2)
                                   (* (+ 100 (- (random 10) 5))
                                      (sin angle)))
                        collect (+ (/ height 2)
                                   (* (+ 100 (- (random 10) 5))
                                      (cos angle))))))
            (format t "Num vertices: ~a~%"
                    (length
                     (sketch::triangulate pts)))
            (with-pen (make-pen :fill img)
              (apply #'polygon pts)))
          (stop-loop))

Output:

Num vertices: 4578
Batch 0

There's only 1 batch of triangles and it works fine.

batching-with-texture

But if num-points is increased to 1500, we get this monstrosity:

Num vertices: 9225
Batch 0
Batch 1

batch-discontinuous

This prevents discontinuities in the texture.

Also removed some of the excess cons-ing when normalizing vertices.
@Kevinpgalligan
Copy link
Contributor Author

Kevinpgalligan commented Apr 23, 2024

In the latest commit, the bounding box is calculated before the vertices are batched, and the issue with the discontinuous texture is fixed. Just gotta come up with a test to show that it works for :triangle-strip.

@Kevinpgalligan
Copy link
Contributor Author

Okay, here's a test to show that batching works for polyline / :triangle-strip!

(defsketch discontinuity
            ((img (load-resource "/path/to/cat.jpeg"))
             (y-axis :up)
             (num-points 10000))
          (let ((pts
                  (loop for i below num-points
                        collect (* width (/ i num-points))
                        collect (/ height 2))))
            (with-pen (make-pen :stroke img :weight 200)
              (apply #'polyline pts)))
          (stop-loop))

Results in 7 batches and this pic:

cat-as-polyline

Can't think of anything else to test, so this fix should be good to go.

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.

1 participant