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

Upstream 4k/dpi awareness Tool fixes #339

Closed
wants to merge 3 commits into from
Closed

Conversation

HarrievG
Copy link
Contributor

Contains all fixes in tool-land to draw and size all window elements in editor windows when DPI settings are used.

Also contains modified cmaketexts. Only SDL part is important now. Keep added settings off until until integration is comple, or take only the changes in tool.

-H.

@DanielGibson
Copy link
Member

Could you rebase this on current master (to get rid of the merge conflicts) and clean it up a bit?
There's lots of unrelated changes in CMakeLists.txt and half of the commits are merge commits (maybe rebasing would already get rid of those?)

@HarrievG
Copy link
Contributor Author

HarrievG commented Jan 16, 2021

Hi Daniel,
Yes will try to rebase, then also the 2 defines in the tool code (.dae) related can go for this base. And it's cleaner in the cmake too. Sorry I am not as git savvy as I want to be ;)

_afxdll: ah yes, MFC ! That is referencing it somewhere, that is why I had a problem with it only when compiling tools

@HarrievG
Copy link
Contributor Author

At some time after the next release I wanna try to get rid of libogg/vorbis(file) and libjpeg and zlib and replace them with header-only libs (stb_vorbis, stb_image and miniz).
So the only remaining dependencies will be SDL2, OpenAL and (optionally) libcurl, and of course the C and C++ standardlibs.
That should make building less painful (though of course especially the combination of SDL2 and CMake might still cause us pain)

I hope I get that release done soon(ish), but there's still some things to do.

To be honest, I also think that I won't merge this change before the release. Don't get me wrong, the 4K fixes for the tools look great and I really want them, but both them and the cmake changes look quite invasive and I don't want to risk breaking anything that worked so far shortly before a release.

Once this branch is in a state that works for me on both Linux and Windows, I'll create a branch based on it (and the then-current master) so you have a sane base for further work (even if I don't get the fscking 1.5.1 release done).

I will take out the casing . No problem. I will also try to setup Travis to build it so I can catch any other build related things. That hopefully will mitigate some risks.

If there is anything I can do to help with 1.5.1 just assign me some bugs and I'll do all that I can.

@DanielGibson
Copy link
Member

DanielGibson commented Jan 17, 2021

If there is anything I can do to help with 1.5.1 just assign me some bugs and I'll do all that I can.

Thanks for the offer!
There's two bugs I'd like to fix before release if it's small/harmless (-looking) fixes: #330 #331 (UPDATE: linked the wrong bug first)
There's also two other bugs that I couldn't reproduce, but that otherwise sound like they should be fixed: #328 #280

Apart from that I'll have to do some work on the mods (https://github.com/dhewm/dhewm3-sdk), like

  • update them with relevant changes done in the dhewm3 repo
  • try to fix Dentonmod crashes (IIRC Rivensin is based on Dentonmod and I already fixed lots of bugs there, I'll have to see what of those fixes apply to Dentonmod and port them)

(but it probably makes most sense if I do the mod-specific stuff myself)

@DanielGibson
Copy link
Member

Oh, another thing that would be nice to have for the next release is #318
I already implemented it for POSIX (Linux etc), but Windows is still missing: https://github.com/DanielGibson/dhewm3/tree/dll-load-errors.

@HarrievG
Copy link
Contributor Author

Yes, let me take those windows specific ones, its my main platform anyway, so i gladly go look at #318 .

@HarrievG
Copy link
Contributor Author

HarrievG commented Apr 21, 2021

Hi Daniel,
was there something else here you wanted to be done before merging?

I want to create pull requests for the debugger too, but it layers on top of this.

Besides that; i see that @turol is refactoring some code which will have impact on the debugger implementation, and i dont want to much divergence :P

cheers!
-H

@DanielGibson
Copy link
Member

Right now this branch can't be merged, the first step would be to rebase it onto the current master branch

@turol
Copy link
Contributor

turol commented Apr 22, 2021

Not really refactoring, #369 is pretty much all of it. It's only a draft because I'm not sure it's a good idea and I know you were also doing something to it. My changes are mostly mechanical and I'm fine with you either incorporating them or doing something totally different.

@HarrievG
Copy link
Contributor Author

@DanielGibson first step complete!?

@DanielGibson
Copy link
Member

Can you please rebase it so we don't have all those "merge master into upstream" commits in the history?

@HarrievG
Copy link
Contributor Author

uhm yes! crap! wrong button :S

@HarrievG
Copy link
Contributor Author

HarrievG commented Apr 22, 2021

not sure what i did wrong now. ( rebase looks correct? )
But it looks like i have some work left:

  • Put back @turol changes ( no idea why they are reverted, auto merge during rebase maybe?)
  • Remove all the QGL, IMGUI and DAE stuff.

@turol
Copy link
Contributor

turol commented Apr 22, 2021

Well that's quite a mess... You're probably better off creating a new branch off the current master and cherry-picking the appropriate commits into it.

Also, https://xkcd.com/1597/

@HarrievG
Copy link
Contributor Author

HarrievG commented Apr 22, 2021

Lol. yeah well. i "remembered" the revert command ;P

I just keep trying to use git as Perforce (Muscle Memory) . But it isnt perforce, and it will never behave like it either, i just have to remember the wise words : "BRANCH FIRST"

Yeah, i will cherry pick.. not giving up!

@DanielGibson
Copy link
Member

DanielGibson commented May 6, 2021

oh shit, I totally didn't think about this when I just merged #376 - I fear there will be merge conflicts (maybe not though, I didn't check)

@HarrievG

This comment has been minimized.

@HarrievG
Copy link
Contributor Author

HarrievG commented May 6, 2021

I cannot reconcile with this. I really have done a lot of work.

So let's try to solve it.

If you help me merging this and getting it in, ( rang me up, walk me through it, really. ), so that my git skills are enough to not have to spend so much time getting it upstream again then everybody wins.

And then I can finally integrate the debugger too. So frigging proud of that one. Not going to let that go to waste, even that Id guy I asked if it was actually allowed to gpl it (because of raven Software) was impressed.

@turol
Copy link
Contributor

turol commented May 6, 2021

I think you almost got it, there's just two extraneous commits remaining. The following should give you a branch without them:

git checkout -b newBranch 9e316ac0d6d9ddb565ca5929d1d9cb35dd92ba94

@DanielGibson

This comment has been minimized.

HarrievG added 2 commits May 6, 2021 23:11
- Nullptr guard/Crashfix in material editor : meMainFrame can be null
  when starting immedeatly from commandline.
- Upped windows version ( needs to go back, and a runtime dpi check comes in place )
- Changed warning settings for msvc to confirm latest compilers
- Fixed up _afxdll
Copy link
Member

@DanielGibson DanielGibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just as a reminder about that get DPI wrapper

neo/tools/radiant/InspectorDialog.cpp Outdated Show resolved Hide resolved
- Use GetDeviceCaps for dpi on anything else than win10.
@DanielGibson
Copy link
Member

I added dynamic loading of the dpi getting function to support older windows versions again, see https://github.com/dhewm/dhewm3/commits/tool-dpi-scaling - if that works I'll merge it.

Win64 binaries with all this: dhewm3-1.5.2pre-win64-tools-dpiscale.zip

@DanielGibson
Copy link
Member

Works on Win7:
100% scaling: dhewm3-100perc
150%: dhewm3-150perc

@HarrievG
Copy link
Contributor Author

Whoo !

Very happy with the end result, thx for adding the win 8 method!

This is now solid as a rock.

😀

@DanielGibson
Copy link
Member

these changes are now in master - thanks a lot! :)

@DanielGibson DanielGibson mentioned this pull request May 10, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants