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

Use tex sizes from texture rather than hardcoded values #610

Merged
merged 11 commits into from
Aug 1, 2022

Conversation

dcvz
Copy link
Contributor

@dcvz dcvz commented Jul 7, 2022

Uses actual size from texture rather than harcoded values. This fixes a few bugs where the textures are actually bigger than the size given (especially when using ASAN).

@dcvz dcvz force-pushed the chore/dynamic-tex-sizes branch from 3815604 to 4090512 Compare July 7, 2022 23:52
@dcvz dcvz marked this pull request as ready for review July 8, 2022 00:33
@@ -199,6 +199,21 @@ extern "C" {
return (char*)res->imageData;
}

uint16_t ResourceMgr_LoadTexWidthByName(char* texPath) {
const auto res = static_cast<Ship::Texture*>(Ship::GlobalCtx2::GetInstance()->GetResourceManager()->LoadResource(texPath).get());
Copy link
Contributor

Choose a reason for hiding this comment

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

this line might be useful as a helper function that gets inlined? it's duplicated in each of these functions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is prime macro real estate.

@dcvz dcvz changed the base branch from develop-rachael to develop-zhora July 25, 2022 19:31
Comment on lines 158 to 172
uint16_t ResourceMgr_LoadTexWidthByName(char* texPath) {
const auto res = LOAD_TEX(texPath);
return res->width;
}

uint16_t ResourceMgr_LoadTexHeightByName(char* texPath) {
const auto res = LOAD_TEX(texPath);
return res->height;
}

uint32_t ResourceMgr_LoadTexSizeByName(char* texPath) {
const auto res = LOAD_TEX(texPath);
return res->imageDataSize;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Would probably be good to have some kind of check to see if res is null, incase the provided path points to a non-existent resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We weren't doing that before but happy to add it. What would be an acceptable recovery here though? Should we just log it out and return -1?

@dcvz dcvz mentioned this pull request Jul 29, 2022
// "Item 32-0"
osSyncPrintf("アイテム32-0\n");
} else {
R_TEXTBOX_ICON_XPOS = R_TEXT_INIT_XPOS - sIconItem24XOffsets[gSaveContext.language];
R_TEXTBOX_ICON_YPOS = y + 10;
R_TEXTBOX_ICON_SIZE = 24;
memcpy((uintptr_t)msgCtx->textboxSegment + MESSAGE_STATIC_TEX_SIZE,
ResourceMgr_LoadTexByName(gItemIcons[itemId]), 0x900);
ResourceMgr_LoadTexByName(gItemIcons[itemId]), ResourceMgr_LoadTexSizeByName(gItemIcons[itemId]));
Copy link
Contributor

Choose a reason for hiding this comment

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

We should avoid memcpy here (and further down) in the first place. The texture should better be loaded in the gfx plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There’s a separate plan to completely remove LoadTexWithName and friends.. but that is a very big change and also requires dealing with the skyboxes. I was aiming to fix some of the issues the arise from textures sometimes being the wrong size than what’s given while that effort happens.

@Kenix3 Kenix3 merged commit 582f084 into HarbourMasters:develop-zhora Aug 1, 2022
@dcvz dcvz deleted the chore/dynamic-tex-sizes branch August 5, 2022 21:52
Kenix3 pushed a commit to Kenix3/Shipwright that referenced this pull request Oct 19, 2022
…rs#610)

* Use tex sizes from texture rather than hardcoded values

* Dynamic do action tex sizes

* Remove unused minimap texture keys

* Restore MESSAGE_STATIC_TEX_SIZE

* Use dynamic offsets

* MACRO it up

* Enable SPDLOG in Xcode

* Handle non-existent texture
dcvz added a commit to briaguya-ai/Shipwright that referenced this pull request Nov 12, 2022
…rs#610)

* Use tex sizes from texture rather than hardcoded values

* Dynamic do action tex sizes

* Remove unused minimap texture keys

* Restore MESSAGE_STATIC_TEX_SIZE

* Use dynamic offsets

* MACRO it up

* Enable SPDLOG in Xcode

* Handle non-existent texture
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants