zturtleman / mm3d

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

MM3D Application does not capture correct mouse location on Retina Macbook #78

Closed orange451 closed 5 years ago

orange451 commented 5 years ago

Hiah. When using MM3D on my retina display macbook, the mouse location that is captured when I click in the editor window is incorrect. For example, in any 3d editor window, if I click at 0,0 and drag my mouse to the bottom right corner (for example 100,100), the mouse position in the editor is at 50,50.

Functionally, everything works the same, it's just that I have to correct for my mouse position by guessing where it captures it. It makes selecting bones/joints a little difficult!

This issue is consistent at any window size, so I suspect you'll have to scale the mouse location by the ratio of the framebuffer size to the window size. This is what I had to do in my own OpenGL rendering engine to correct for a retina display.

orange451 commented 5 years ago

This is probably a totally new problem, since before you took over MM3D, I don't think anyone ever released a mac binary! If any more info is needed, let me know.

zturtleman commented 5 years ago

I am able to reproduce the issue on GNU/Linux by setting QT_SCALE_FACTOR environment variable to 2 (e.g. QT_SCALE_FACTOR=2 mm3d in terminal).

zturtleman commented 5 years ago

Mouse input with high-DPI windows should be fixed now and I increased the size of lines and points (selection box, wireframe, ...) for high-DPI windows to match 'regular DPI' windows. https://github.com/zturtleman/mm3d/compare/1282798bbef7a72896cf0d1beff82c8f0b7eff8f...f3aca1d055d209f46771151437e9b6d1767de81f.

m-7761 commented 5 years ago

That commit makes me really uneasy.

What is the problem here? Doesn't Retina change OpenGL to render larger? Is it hard to see the lines if they are rendered at 1px?

In any case, I'm doing some quality work to better organize modelviewport.cc right now. I think this commit is going to be a problem to merge into that work. But I don't really understand the situation with the Retina, or high DPI.

I've changed ModelViewport to not depend on Qt constructs, and to use OpenGL coordinate system.

If for some reason high-DPI needs to change the thickness of lines, I recommend setting a multiplier (will integer not do?) inside Model, and getting it from there for anything that Model doesn't draw itself. It's too sloppy to pass around and alter these APIs. It has to be passed everywhere for no reason, the wrong thing can be passed. Not all code can query Qt.

m-7761 commented 5 years ago

In the shower it occurred to me a better way is to just set glLineWidth and glPointSize when the context is initialized. E.g. initializeGL.

The only place the code uses a wider width is drawing the seam lines of projections. Those could be drawn in a different color, or glGet can get the base width, change it to a multiple, and restore it after.

(In any event, this is how I would implement this myself. The implui code should set these values either once when the GL context is made, or before drawing the ModelViewport and TextureWidget elements.)

I confess I still don't know enough about the Retina shenanigans. Does width need to larger or smaller? I.e. is it compensating or undoing something that is compensating.

zturtleman commented 5 years ago

Response to first reply

High DPI / Retina displays typically have 2x resolution but expect applications to use half resolution for logical UI sizing but with higher resolution text, icons, and 3D model viewports (Qt devicePixelRatioF() would be 2.0). In Windows 10 you can set the text scale to 175% (which is 1.75 DPI scale) so integer scaling is not sufficient. Qt supports both of these situations.

By default 1px lines would appear half size on a high DPI / Retina display. I don't have a high DPI display but I would imagine it's harder to see what would be 0.5px line on my current 1080p display. Increasing the line width provides the same relative size and should look better than standard DPI due to higher resolution of the model viewport. This seems to be the intention of high DPI.

The window (and thus ModelViewport) DPI scale is unrelated to the model data. It may change at any time (e.g., moving from high DPI monitor to a standard DPI monitor). DPI scale is not specific to Qt. Setting DPI scale on Model class each time a ModelViewport is drawn (in case the DPI scale changed) so draw functions can indirectly be passed an argument seems sloppy to me. Maybe it would make sense to store it in ModelViewport and have whatever UI toolkit set it. (Maybe DPI scale change occur with a Qt resizeGL(). I'm not sure.)

[paraphrase] "Don't store render state in Model class. It belong to the ModelViewport!" "Don't pass render state to Model class methods. That's sloppy! Add to Model class instead." :thinking:


Response to second reply

Yeah, that's a decent idea. Model::drawVertices() has unselected vertexes be size 3 and selected be size 4. I don't see why they should become the same size because points now use DPI scale. Could get size from OpenGL and multiply, yes.

m-7761 commented 5 years ago

If I switch a program to a new monitor with a different DPI I don't expect it to survive... I don't expect it to even be renderable if I didn't start the program there. I think it's a lowest-priority goal to be able to do that. Users shouldn't be doing that. They would probably only do it to see if it works or not for giggles.

So, I wouldn't make any design decisions to that end, period. It's much lower priority than any design decision.

That said, I haven't reviewed the entire commit, and I don't understand the relationship to the mouse coordinates. But it sounds like OpenGL is somehow unaware of the Retina system? I think I've seen elsewhere that many software on Retina systems have the opposite problem, i.e. Apple's OpenGL under the hood changing their values. That makes sense to me since most software is probably not Retina aware.

Yeah, that's a decent idea. Model::drawVertices() has unselected vertexes be size 3 and selected be size 4. I don't see why they should become the same size because points now use DPI scale. Could get size from OpenGL and multiply, yes.

I may have read that wrong, but it seemed more like to me it was just tidying up by restoring the point-size to 4, as-if that's the default. I could be wrong. There is also some code that weirdly sets selected edges to 1.6 width. I'm pretty sure that doesn't make a difference. It might only for diagonal lines, or subpixel differences. Obviously, that should not be that way.

[paraphrase] "Don't store render state in Model class. It belong to the ModelViewport!" "Don't pass render state to Model class methods. That's sloppy! Add to Model class instead." 🤔

You're probably misunderstanding these tickets. The first is just a note that the macro is not used consistently, and so doesn't work. I don't have a problem with it if it's consistent, but it does seem like the Model class is just a little overloaded in general. Those states are in Model because it's the gatekeeper for the undo system, and they need to have undo/redo logic. I'm pretty sure otherwise they wouldn't be shoehorned into it.

As for Right/Left, I argue that the cleaner solution is just to reverse the sense at the Model level. Model doesn't actually use those at all, so that really just means at the ModelViewport level, which I've done. It was pretty trivial. I could share my "depui" files I'm working with, if you are interested in trying to integrate them. I think they are pretty well stable, but I haven't tested the texture windows yet. There are 4 in all, 2 of which are headers. They need to be adopted and moved into mm3dcore if we are too keep collaborating on the non-UI parts. I think that is the best outcome, but developers often can't collaborate.

There is already much render-state in Model. I think there should be more (https://github.com/zturtleman/mm3d/issues/56) so I don't see a flaw in storing DPI in Model. It's really just a data sharing mechanism. It's the glue between the different components. But I do prefer having the UI code just set glLineWidth, etc if that's the extent of it.

In my modelviewport.cc code it only knows about m_viewportWidth etc. It adds m_viewportX and Y also. It doesn't have a GL context of its own. The UI class is responsible for setting its origin and dimensions and calling its draw routine, just like Model. That's how it can be moved out of depui. Which I think is important because it's not a trivial class. Everything else in implui is as streamlined as can possibly be. But depui has lots of code that has no relationship to the UI system, so it's a waste to keep bound to the UI system. And it would be a chore to ask someone to reimplement. Just like you wouldn't want to reimplement Model because it is embedded in a proprietary UI system. In other words, apart from OpenGL dependency, it can be strictly portable code, and so should be.

I've likewise removed the UI and configuration code from the "tools" code. I think it's good to make these components reusable. One of the first things I did is rip out MM3D's config code. It's very awkward and inefficient. So I recommend either it be moved into implui, which is the only place that uses it, or I recommend better to see if the UI framework (e.g. Qt) offers a portable config system. Better to that if so.

Anyway, I hate to be a PITA, but I recommend trying to see if this change for Retina, etc. can be simplified so we can easily splice these changes before very long.

Here is a new screenshot of my progress. I'm having to build out a UI framework as I go, which is why it's a little slow-going. I made it to give to @orange451 when we met on Discord. They are supposed to be preparing a model pack or something. It didn't materialize that day, but I'm confident it will.

Widgets 95 MM3D 2

P.S. They actually knew something about the source material for my game project, which was pretty weird, because it's super obscure stuff.

zturtleman commented 5 years ago

I have three main goals for Maverick: Work on modern operating systems. Support model formats supported by ioquake3 (MD3, MDR, IQM/IQE). And support SMD and MD5 model formats to usable for other 2000s game engines. I've been slacking off on the model formats though.

Collaboration is difficult because I don't really have any interest in doing the actual work for deep redesign and improvement of libmm3d/mm3dcore, editor, or UI. It gets the job done. Replacing Qt for a UI toolkit (Widgets 95) with a single developer seems like a risky idea in the long run to me. So it's difficult to be completely on board with your revamped version being a replacement for Misfit/Maverick (though I guess it's mainly wxWidgets so maybe not that big of deal?).

The idea that Maverick is hospice directly caring Misfit Model 3D 1.3.8 and you're creating a better life-improving 3D modeler inspired by Misfit Model 3D for the masses is fine by me (it's excellent really). I appreciate your bug reports. Though I don't particularly know what to do with the code / UI design questions. Somehow it ends up needlessly antagonistic feeling.

If you're not going to submit tested pull requests to Maverick, I'd just assume wait for Daedalus 3D source code to be released and deal with or not deal with reconciling libmm3d/mm3dcore differences of my own accord. (It's not something I want to work on right now so I don't need an early copy.) Maverick's libmm3d/mm3dcore will be barely changing so it shouldn't be a big issue, right?

My future goals for Maverick 1.5 (OpenGL ES 2 compatible renderer, glTF 2) will start as separate engine and model utility projects. At this point I think it's unlikely I'll integrate them into Maverick.

m-7761 commented 5 years ago

I don't understand why you marked most of these posts off-topic, since they are very much on topic, but FWIW, my thinking is it's for the best, as MM3D is open-source, to compartmentalize the components, since they are already clearly intended to be so compartmentalized.

Cleanly separating the implui code from the rest furthers that end. It doesn't have anything to do with switching the UI framework, so much as separating the UI element, so that implementations may overlap and benefit from having the other components be compatible.

Different applications can then reuse the code, or at least more easily integrate changes. Just like Assimp for example is used by many applications. It's not such a crazy idea. My problem is just by choosing to not use Qt, I can't contribute work if the components are not made to be divorced.

That's the extent to which I'm interested, and if I'm being honest, my interest is strictly ethical; that is to say that I have a duty to at least attempt to be generous and also give and not just take; or at least that is my personality. It's not good when code splinters. But it's often unavoidable since people are unwilling or unable to meet halfway.

Maverick's libmm3d/mm3dcore will be barely changing so it shouldn't be a big issue, right?

EDITED: Not for the first round of releases, but if Maverick is changing in the meantime it will complicate the matter. That's why it's worthwhile to discuss any changes like this one. At some point Daedalus 3D will split as a completely different project. I may refactor to avoid GPL stuff to the extent I can. Right now I'm trying to give back to MM3D's legacy by making as much of my upgrade as possible available to it. And I will prepare an unadvertised version of it that will stick to using MM3D format, and call it something else.

zturtleman commented 4 years ago

They were marked as off topic because they are not about Maverick supporting mouse position or drawing lines on high DPI / Retina displays. It's about integrating / reconciling changes with your private fork. Which I think is an entirely separate issue.