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

Refactor and improve player map (F11) #3823

Merged
merged 37 commits into from
Nov 21, 2024

Conversation

Fernando-A-Rocha
Copy link
Contributor

@Fernando-A-Rocha Fernando-A-Rocha commented Oct 23, 2024

This PR:

  • Refactors and Renames Radar to Player Map in the code, so it can't be confused with the minimap radar (as it is called in Singleplayer), leaving only the commands for backwards compatibility
  • Repositions and translates the F11 help text strings
  • Replaces the F11 Map radar.jpg image with a better image of the same resolution (1024x1024) converted to PNG for optimization (Source)
  • Adds a new 2048x2048 PNG map image (Source)
  • Adds a new Setting for switching between the two images (the 1024 pixels image remains the default one)

image

Closes #2904

@TracerDS
Copy link
Contributor

What do you think about 4k or 8k instead of 2k?

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 23, 2024

4k has good level of detail, 8k is awesome but imo it has too many pixels. Idk how most hardware would handle a higher image resolution (it's more intensive to render).
maybe 2k is fine

@TracerDS
Copy link
Contributor

4k would be sufficient for now, though more feedback would be nice.
It wouldnt be a problem for me but maybe it would for other users as the size doubles with each dimension

@Fernando-A-Rocha Fernando-A-Rocha changed the title Remake/upscale radar.jpg (1024px -> 2048px) Remake/upscale radar.jpg Oct 23, 2024
@FileEX
Copy link
Contributor

FileEX commented Oct 23, 2024

Let's not exaggerate, an 8k map, while the entire game uses textures at most 512 or 1024, sounds absurd. 4k will be more than sufficient. It's not something people will stare at and admire, like 'wow, what an amazing map!' because everyone knows it by heart. They'll click on it in a few seconds, and no one will pay attention to its quality

@Fernando-A-Rocha
Copy link
Contributor Author

Let's not exaggerate, an 8k map, while the entire game uses textures at most 512 or 1024, sounds absurd. 4k will be more than sufficient. It's not something people will stare at and admire, like 'wow, what an amazing map!' because everyone knows it by heart. They'll click on it in a few seconds, and no one will pay attention to its quality

do you suggest 2k or 4k?

@FileEX
Copy link
Contributor

FileEX commented Oct 23, 2024

Imo, 4k is the maximum size we should consider

@TheNormalnij TheNormalnij added the enhancement New feature or request label Oct 23, 2024
@TheNormalnij TheNormalnij added this to the 1.6.1 milestone Oct 23, 2024
@TheNormalnij
Copy link
Member

TheNormalnij commented Oct 23, 2024

Imo, 4k is the maximum size we should consider

2k is the safe maximum. #1051

@TheNormalnij
Copy link
Member

@Patrick2562, can we license your work under GPL3?

@T-MaxWiese-T
Copy link

What would also be cool is if the map would adapt to the time of day, so when it's dark the night map is displayed. But I think just like the chat is planned to be outsourced to a resource, the map should also be outsourced to a resource where you have settings.

@Dutchman101
Copy link
Member

This can only be considered for merge if all the feedback from PR #1051 is implemented, so please carefully read the comments there (from team members: ccw, me, and qaisjp at the time). In short:

  • If the remake/upscale uses textures larger than 2048 x 2048, it must be opt-in (not opt out) with a popup disclaimer that enabling it can cause issues with certain Intel GMA/HD graphics chips, and crashes due to the load, as we have limited VRAM in GTA by design.

Also, even before rendering the image (user pressing F11) it will cause a passive extra load on (V)RAM, just because it gets loaded.

@Patrick2562
Copy link
Member

@Patrick2562, can we license your work under GPL3?

Yes, of course

@Fernando-A-Rocha
Copy link
Contributor Author

This can only be considered for merge if all the feedback from PR #1051 is implemented, so please carefully read the comments there (from team members: ccw, me, and qaisjp at the time). In short:

  • If the remake/upscale uses textures larger than 2048 x 2048, it must be opt-in (not opt out) with a popup disclaimer that enabling it can cause issues with certain Intel GMA/HD graphics chips, and crashes due to the load, as we have limited VRAM in GTA by design.

Also, even before rendering the image (user pressing F11) it will cause a passive extra load on (V)RAM, just because it gets loaded.

Right now this PR includes a 2048 x 2048 image, which abides by those ideas afaik

@Fernando-A-Rocha
Copy link
Contributor Author

Should we switch to png?

@TracerDS
Copy link
Contributor

png would weight more but it would also improve the quality of the image

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 23, 2024

I'll edit Client/mods/deathmatch/logic/CRadarMap.cpp with the new image size accordingly

@Fernando-A-Rocha
Copy link
Contributor Author

We could also change the top and bottom white text's position/size/etc. to make it prettier. What do you guys think?

image

@TracerDS
Copy link
Contributor

Adding a border/shadow to the text would be enough i think.

Also that image looks... different with such scaling. It would need to be scaled down to fit on the screen/stretch it out (maybe not)

@Fernando-A-Rocha
Copy link
Contributor Author

Adding a border/shadow to the text would be enough i think.

Also that image looks... different with such scaling. It would need to be scaled down to fit on the screen/stretch it out (maybe not)

I zoomed in to take a screenshot xD The size looks the same IG

@Fernando-A-Rocha Fernando-A-Rocha changed the title Remake/upscale radar.jpg Improve radar map (F11) Oct 25, 2024
@TheNormalnij
Copy link
Member

Do you request an option in the MTA settings menu? @Dutchman101

@Dutchman101
Copy link
Member

Dutchman101 commented Oct 26, 2024

png would weight more but it would also improve the quality of the image

It's only weigh in size, in fact png is a more optimized format by its library (less of a performance hit in rendering, for equal resolution & texture, regardless of file size) and is more well-maintained over libjpeg, which is basically ancient and not modern. So it should be PNG for this PR.
@TracerDS

Do you request an option in the MTA settings menu? @Dutchman101

Yes, i think that since this doubles the texture res, it should be opt-in like described earlier.. it'll generate a performance hit regardless of being low enough to avoid those Intel GPU limits. Either in the video settings or advanced settings tab. In this PR, you can also exchange the standard texture (1024 x 1024) to your new texture, but just in a 50% upscale way to make it also 1024 x 1024, to keep visual parity.
@Fernando-A-Rocha

So, if you follow all requests, simply put: you'd end up pushing 2 PNG's, one of 1024 and one of 2048 (getting rid of the current jpg one in the process).

@Fernando-A-Rocha
Copy link
Contributor Author

All right. Do you have a suggestion for the best way to convert both the current and new jpg images to png? @Dutchman101

@Fernando-A-Rocha Fernando-A-Rocha changed the title Improve radar map (F11) Improve player map (F11) Oct 30, 2024
@Fernando-A-Rocha Fernando-A-Rocha changed the title Improve player map (F11) Refactor and improve player map (F11) Oct 30, 2024
@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Oct 30, 2024

@TheNormalnij we still have the issue of addressing CreateTexture failure which can happen for:

  • map image
  • player blip image
  • map blip images

After all these years, I couldn't find a report of someone complaining textures failed to create.

Still, what solution do you propose?

@Fernando-A-Rocha
Copy link
Contributor Author

Fernando-A-Rocha commented Nov 11, 2024

@TheNormalnij we still have the issue of addressing CreateTexture failure which can happen for:

  • map image
  • player blip image
  • map blip images

After all these years, I couldn't find a report of someone complaining textures failed to create.

Still, what solution do you propose?

i made the code handle texture creation in these cases.

This PR is ready.

@Dutchman101
Copy link
Member

This PR is ready.

I agree, given how many code reviews you passed, and you've incorporated all feedback as well.
Thanks for improving the map!

@Dutchman101 Dutchman101 merged commit 2c5cf32 into multitheftauto:master Nov 21, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update default radar.jpg image (F11 map)
8 participants