Bug detected in DebugRenderer::Render method

While playing with some widgets based on DebugRenderer’s DrawTriangle method, I noticed that depth testing was failing for debugdraw solids - to be clear, they are properly depth tested against scene geometry, but NOT each other!
When I examined the sourcecode for DebugRenderer:Render method, I noticed that, for some silly reason, depth write is being disabled immediately prior to drawing depthtested triangles!
I moved the offending line of code slightly futher down, to just prior to non-depth triangles, this completely fixes the problem, debug solids will be properly depth-sorted.

3 Likes

Could you turn this into a pull request?

It’s hardly worth the effort!

Very near the end of DebugRenderer::Render method,

    graphics->SetBlendMode(BLEND_ALPHA);
    // graphics->SetDepthWrite(false);              /// MOVED THIS

    if (triangles_.Size())
    {
        count = triangles_.Size() * 3;
        graphics->SetDepthTest(CMP_LESSEQUAL);
        graphics->Draw(TRIANGLE_LIST, start, count);
        start += count;
    }
    
    graphics->SetDepthWrite(false);                 /// TO HERE
    if (noDepthTriangles_.Size())
    {

Apparently, I can’t turn it into a pull request just yet, as my first PR is still being evaluated… I’m denied from issuing another PR under the same credentials until this one is sorted, I gather.

1 Like

That’s nonsense. One-liners are more likely to pass without much consideration.
Especially if they fix bugs.

I am delimited from issuing PR until the current PR is cleared, it appears

I have over 60 changes to push now, and I can’t issue PR, to the master, under my current credentials

You can submit multiple PRs. They just have to be on different branches

1 Like

So every time I want to issue PR I need another branch, until or unless someone plays catch up? sounds enticing

I recently did a one-line PR. It was merged the same day. The same could be true for your improvement to the DebugRenderer. The amount of simultaneous changes greatly defines how much effort it takes others to make sense them

1 Like

Personally, I have no objections to having multiple branches. If you do, I’ll offer a benefit to multiple branches that I’ve experienced: it makes it easier to bring your code into line with the master branch if you have merge conflicts. I have my own personal branch with a couple of changes that have not been merged into master, and I can easily have a dozen files with merge conflicts when updating to the latest master. Using branches, it’s easy to just update the branch that follows Urho’s master to head, then branch and make the couple line changes without having to merge all the other changes with the new changes that were pulled. It’s then just pushing to my GitHub account and creating a new pull request, which as Modanung pointed out, is then very easy to review when it’s a couple lines fixing some bug.

3 Likes

I appreciate that merging, and testing, is not trivial - even when it can be partially automated. I’d have preferred to PR atomically for each change I make, but given that the first one I ever offered is still languishing, I chose to put them up on a cloned repo, and occasionally shuffle them across to my fork. This puts a much greater load on those who are meant to be handling the merging, I know, but if my first atomic PR is still languishing, why should I expect branched atomic PR not to languish also?
Personally, I don’t stand to gain anything, whether people can see my changes, or not, but I did offer to donate some time to remedy some outstanding issues, and will continue to document and remedy issues, and push them Somewhere Public - I’ll also create a branch or two, because my changes tend to be erratic and not directed at one thing at one time, but rather what I stumble across from day to day. I mean that, when I say “from day to day”, I would like to be issuing PR almost daily, but there’s no sign that our current crew can keep up.

In an ideal world you’d test your unfounded biases. In cases like these it takes you and the “current crew” the same amount of effort as when not making PRs, while having a more productive result.

I do test each and every thing I would propose, it vexes me that you would imply I do not, it would insinuate I was unprofessional in my approach, which would also reflect on the establishments that provided me with my qualifications, plural.
You do not recognize my qualifications - personally?

I ask for third party testing, and provide full source, annotated with comments, what more can I do?

Bla, bla, bla…
PR?

I will branch PR as I have no alternative, but I may think twice about what I care to share, ok with you?

Fortify your beliefs wherever you like. That does not make them accurate.

This is not how its meant to be. Sigh.

My bug fixes are accurate. You’re merely setting formalities for sharing of information, and then stepping on it. I do not agree with your process.