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

Fix raster overlay undefined behavior #1554

Merged
merged 8 commits into from
Nov 25, 2024
Merged

Fix raster overlay undefined behavior #1554

merged 8 commits into from
Nov 25, 2024

Conversation

kring
Copy link
Member

@kring kring commented Nov 20, 2024

This is a PR into #1552 so merge that first.
This depends on CesiumGS/cesium-native#1003 so merge that first, too.

Fixes #1551

@kring kring added this to the December 2024 Release milestone Nov 20, 2024
Base automatically changed from update-native to main November 20, 2024 20:42
@azrogers
Copy link
Contributor

Trying to test this and CesiumGS/cesium-native#1003 out and I'm getting a lot of errors relating to the changes in main, particularly over gsl::span:

1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(29): error C2653: 'gsl': is not a class or namespace name
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(29): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(29): error C7568: argument list missing after assumed function template 'span'
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(29): error C2144: syntax error: 'unknown-type' should be preceded by ')'
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(29): error C2144: syntax error: 'unknown-type' should be preceded by ';'
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(29): error C2059: syntax error: ')'
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(29): error C2433: 'UnrealAssetAccessor::contentPayload': 'virtual' not permitted on data declarations
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(24): warning C4263: 'CesiumAsync::Future<std::shared_ptr<CesiumAsync::IAssetRequest>> UnrealAssetAccessor::request(const CesiumAsync::AsyncSystem &,const std::string &,const std::string &,const std::vector<CesiumAsync::IAssetAccessor::THeader,std::allocator<CesiumAsync::IAssetAccessor::THeader>> &,const int)': member function does not override any base class virtual member function
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\CesiumRuntime\Public\UnrealAssetAccessor.h(41): warning C4264: 'CesiumAsync::Future<std::shared_ptr<CesiumAsync::IAssetRequest>> CesiumAsync::IAssetAccessor::request(const CesiumAsync::AsyncSystem &,const std::string &,const std::string &,const std::vector<CesiumAsync::IAssetAccessor::THeader,std::allocator<CesiumAsync::IAssetAccessor::THeader>> &,const std::span<const std::byte,18446744073709551615> &)': no override available for virtual member function from base 'CesiumAsync::IAssetAccessor'; function is hidden
1>C:\Dev\prs\cesium-native-1003\Plugins\cesium-unreal\Source\ThirdParty\include\CesiumAsync\IAssetAccessor.h(57): note: see declaration of 'CesiumAsync::IAssetAccessor::request'

Does this PR then also depend on #1539? It seemed to be non-trivial getting the ue55 branch merged into this one to test it out, so I didn't go that far.

@kring kring changed the base branch from main to ue55 November 24, 2024 21:02
@kring
Copy link
Member Author

kring commented Nov 24, 2024

Yeah, after the C++20 upgrade PRs were merged into cesium-native main, and main was merged into CesiumGS/cesium-native#1003, this became dependent on #1539. I merged that branch into here and changed the base branch accordingly.

@azrogers
Copy link
Contributor

Looks great! Thanks @kring!

@azrogers azrogers merged commit 2cc7f7e into ue55 Nov 25, 2024
30 checks passed
@azrogers azrogers deleted the fix-raster-overlay-ub branch November 25, 2024 22:38
kring added a commit that referenced this pull request Nov 26, 2024
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.

CesiumPolygonRasterOverlay causes issues with multiple raster overlays
3 participants