zturtleman / mm3d

Maverick Model 3D is a 3D model editor and animator for games.
https://clover.moe/mm3d
GNU General Public License v2.0
114 stars 22 forks source link

Fix? Persp view is 2x larger than Ortho #99

Open m-7761 opened 4 years ago

m-7761 commented 4 years ago

Update: Never mind this fix. See comment.

Changing the code in modelviewport.cc like so (2x zoom for persp) makes the modes line up almost exactly. I don't see a divergence over large/small zoom factors.

Unfortunately I can't explain why, but just by looking you can see something is wrong. This may not be the best fix, but generally the perspective projection matrix using a FOV setup (as is the case) can't be scaled otherwise.

        viewPoint[0] = (float)m_scroll[0]; 
        viewPoint[1] = (float)m_scroll[1]; 
        viewPoint[2] = (float)m_scroll[2]; 

        if(m_view<=Tool::ViewPerspective)
        {
            viewPoint[2]+=m_zoom*4; //WRONG (See 2 comments down code)

            glTranslatef(-viewPoint[0],-viewPoint[1],-viewPoint[2]);
        }
        else 
        {
            viewPoint[2]+=m_zoom*2;

            //https://github.com/zturtleman/mm3d/issues/97
            //glTranslatef(0,0,-m_farOrtho/2);
            glTranslatef(0,0,-(m_farOrtho+m_nearOrtho)/2);
        }
m-7761 commented 4 years ago

EDITED: I spoke too soon. That fix just happened to work for the 2x1 view. The problem seems to be aspect ratio dependent. I will try to figure it out later. If Ortho/Persp don't match it's impractical to switch between them to do work.

I think the fix may be as simple as adjusting according to the aspect ratio.

m-7761 commented 4 years ago

This is the magic formula. 1.2 appears in multiple places throughout the modelviewport.cc file. I'm guessing it should be a defined constant.

        if(m_view<=Tool::ViewPerspective)
        {
            //https://github.com/zturtleman/mm3d/issues/99
            if(aspect<=1)
            viewPoint[2]+=m_zoom*2*1.2/aspect;
            else
            viewPoint[2]+=m_zoom*2*1.2;

            glTranslatef(-viewPoint[0],-viewPoint[1],-viewPoint[2]);
        }

P.S. The reason <= is used is I've set up a user view system that lets custom persp/ortho views be made. The persp ones are from 0 down. And ortho from Ortho up. I've set up a session memory system which copies settings over based on the direction/projection modes. With the custom ones, you (users) can associate them across view layouts. (That is, if you need more than 1 of each that don't clobber each other on layout change.)

(I've set up a lot of convenience features of late.)

m-7761 commented 4 years ago

Off-topic: I've partly implemented tool work in Perspective mode. It became a bit of a nail biting project (not literally) since I thought it would be simple, and so didn't approach like a multi-hour project. What eventually really tripped me up is after it didn't go smoothly I looked at the gluProject documentation, which doesn't include a homogeneous-divide step so I took it to be unnecessary.

I regrouped after a nights rest and set up a visualization, which helped to simplify things. The night before I got super lucky and happened upon specific homogeneous-coordinates for drawing the selection rectangle in the SelectTool code, but I think it will be a wash ultimately. Sorry to make a story out this. -0.2 z and 0.25 homogeneous factor. I don't even know what those could be, but after like 3 tries those worked perfectly, like an angel had taken pity on me.

It would be simpler to draw the rectangle in screen-coordinates instead of with the inverse-projection matrix. Part of the problem about how to proceed is it's unclear view+projection is appropriate for all tool operations, so it requires some kind of survey and plan to take to the next level.

Long story short, I've changed the nulltool to be like a "no tool" (None) mode that lets you manipulate the view, like in Perspective mode, and provides and animation slider. So perspective mode is no longer special in that case. So what I ended up with is if the selection option is enabled to be able to use tools in Perspective mode. So you have options to either switch to the "null tool" or toggle the selection option to change how to manipulate the view. I think it's frustrating when you can see the selection but can't click on it.

To give visual cues, I've hidden the vertices with the "null tool". I've assigned Tab to toggle between the tool and "null" (no) tool, and Spacebar toggles between the current and previous tools, excluding the "null" tool. Escape (Esc) I've assigned to chose the "null tool" but I don't know if that's a long term best use of Esc or not. Using Tab makes it a little like Blender since toggling the tool on/off is a little like entering/exiting an editing mode. I think it would be an improvement (though a little confusing) to make undo/redo apply to view changes when selecting the "null tool" so that you can revert camera mishaps, that are often frustrating in software that can't... which is most software. I made an icon for it, that looks like a clapperboard, that turned out pretty nice. The board part is like a roll of film, so it suggest audio/video mode, or film director mode. I think of it as read-only viewer mode, but there is no good word for it, so I call it None.

EDITED: Here is an illustration of that icon... Widgets 95 MM3D 4

m-7761 commented 4 years ago

FWIW how I resolved the projection for tools problem was to make non-selection tools use the orthographic matrix, which matches close enough with the changes described in the top of this issue.

I'm also able to say that as of now I've settled on the features for the first release, and am moving on to finishing up the CMake scripts and must do a GCC/Linux debugging phase before I can publish anything preliminary. I have to slightly refactor my files since I started work using a forced-include header, just to ease the Qt port work.

I want to publish something so I can take a break. But my findings is Kevin's code is needlessly written to perform badly. (I think he had a sadistic relationship with his CPU.) So I know I will have to return to do another big round of work on the mm3dcore component, which could make things even harder to reintegrate.

m-7761 commented 4 years ago

I couldn't relate the mystery factor 1.2 to anything. The other use of 1.2 is arbitrary to add margins. It doesn't correspond to using 1.2 to balance the ortho/persp views. Note, it balances perfectly for a character model. I don't think scale or meter has to do with it. But it might. My test file is 1=1m. The model_equal_test model doesn't balance so perfectly, but it's still a better fit. I don't know if it's because it's large compared to the grid (doubt it) or if it's just because of it's oblong shape. I don't see it breaking down at extreme zoom levels.

I guess it can only related to the 45 degree field of view. I don't know. It seems odd that is just happens to match 1.2 used by the margins, which is the only reason I chose it. But fortuitous I suppose.

I just looked at frameArea that is where the other 1.2 is used. I ended up radically simplifying it for better results. Most of its code I think is unwarranted.

EDITED: I think the lock logic needs more thought since I've opted to lock the ortho/persp views... so I've omitted its code.

void ModelViewport::frameArea(bool lock, double x1, double y1, double z1, double x2, double y2, double z2)
{
    if(lock) //2019
    //if(m_view<Tool::ViewOrtho
    // &&m_view>Tool::ViewPerspective)
    {
        //HACK: This is just avoiding locking if 
        //there is less than two 2D views. 

/*...*/ //OMITTED
    }

    m_scroll[0] = (x1+x2)/2;
    m_scroll[1] = (y1+y2)/2;
    m_scroll[2] = (z1+z2)/2;

    double width = std::max(fabs(x1-x2),0.0001667);
    double height = std::max(fabs(y1-y2),0.0001667);
    double depth = std::max(fabs(z1-z2),0.0001667);

    Vector bounds(width,height,depth); 

    bounds.scale(1.2);

    m_viewMatrix.apply3(m_scroll);
    m_viewMatrix.apply3(bounds);

    width = fabs(bounds[0]); height = fabs(bounds[1]);

    //NEW: Make zoom consistent for all viewports? Certainly
    //looks better?
    if(lock) width = std::max(width,fabs(bounds[2]));

    m_zoom = std::max(width,height)/2;  

    //QString zoomStr;
    //zoomStr.sprintf("%f",m_zoom); 
    //emit zoomLevelChanged(zoomStr);
    //emit(this,zoomLevelChanged,m_zoom);
    parent->zoomLevelChangedEvent(*this);

    updateMatrix(); //NEW
}