Eric Anholt <[email protected]> writes: > concurrent Thanks for the review, Eric. I'll take care of little things like "concurrent" etc. throughout, as you mention.
> It would be nice to verify that the correct number of fragments is > actually generated, not just != 0. It's not obvious to me without > reading the spec what the right answer would be for glBitmap(), for > example. Yes, that would be a nice improvement to the tests. I don't know the actual correct numbers myself, (though I guess I can just sample the current implementation and see if the numbers make sense). It's interesting that you should mention glBitmap. My reading of the spec. suggests it should yield a non-zero result, but the current implementation on master, (without any of my patches), is yielding a value of 0 for this operation. I haven't looked closer to see what's going on there, but I'd be glad if anyone has any ideas. >> + glBindFramebuffer(GL_READ_FRAMEBUFFER, fb); >> + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); >> + glBlitFramebuffer(0, 0, 2, 2, >> + 2, 2, 20, 20, >> + GL_COLOR_BUFFER_BIT, GL_NEAREST); > > Ah, it's done this way to specifically hit the meta path? That might > deserve mention, since I was about to suggest copying from the window to > the window to get the job done. No, you're giving me too much credit. You're mostly just seeing me thrashing around trying to learn how to use GL. I saw "BlitFramebuffer" and likely just mistakenly assumed a needed to do a bunch of "BindFramebuffer" stuff to set it up. So I'll see if I can't simplify things here. >> + /* Paint the copied texture just ensure it worked. */ >> + glEnable(GL_TEXTURE_2D); >> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); >> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); >> + piglit_draw_rect_tex(22, 2, 18, 18, 0, 0, 1, 1); > > If we're not probing the results, it's not much of an ensuring it worked > :P Right. The test isn't verifying that. I meant more that I wanted to put the results on the framebuffer so that I could manually verify that the test was doing what I expected it to. Let me know if you think any of this should be removed. (I don't think adding any probing would improve the test, but it does seem like making the results visible could help anyone trying to decipher the test in the future. Perhaps I should just update the comment along those lines.) Anyway, thanks for all the review. I'll update the series according to all your comments and post that in the morning, (and then probably just push it unless there's more immediate feedback). -Carl
pgpR29OI3yb26.pgp
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
