PCG data gets corrupted by Copy Points

Development Programming & Scripting
, ,
1

In some PCG graph the data gets corrupted in a way that makes it misbehave.

In my example it causes Copy Attributes to not work correctly.

I think the issue is caused by Copy Pointsbut but I’m not sure.

Tested in stock UE 5.7.4

[Attachment Removed]

2

Steps to Reproduce

Create a PCG graph as follow:

[Image Removed]

Observe that the output data of Copy Attributes doesn’t match the data the input data

[Attachment Removed]

3

Hi Andre,

Thanks for reaching out! We weren’t aware of this, and this is still actually true in 5.8 so we’ll fix it asap!

I’ll come back to you when we have a proper fix so you can cherrypick it yourself.

Cheers,

Julien

[Attachment Removed]

4

Hi André!

Thanks a lot for the report, we found the issue and we’ll fix it in 5.8.

There are two things there, when copying metadata we were not handling the parenting correctly.

The first fix, and what will unblock you is this:

In PCGMetadataHelpers.cpp, at line 213 in 5.7,

replace

if (FPCGMetadataAttributeBase* NewAttr = TargetMetadataDomain->CopyAttribute(SourceAttribute, LocalDestinationAttribute.Name, /*bKeepParent=*/false, /*bCopyEntries=*/bCopyEntries, /*bCopyValues=*/true))by

if (FPCGMetadataAttributeBase* NewAttr = TargetMetadataDomain->CopyAttribute(SourceAttribute, LocalDestinationAttribute.Name, /*bKeepParent=*/bCopyEntries, /*bCopyEntries=*/bCopyEntries, /*bCopyValues=*/true))We’ll investigate where this could happen elsewhere in the engine, and see if we can come up with a fix that will catch all the usecases.

If you have any other questions don’t hesitate.

Have a great weekend

Adrien

[Attachment Removed]

5

That seems to have fixed the issues.

Thanks for looking into it and coming up with a fix, this is perfect!

[Attachment Removed]

6

Glad to hear that!

[Attachment Removed]

7

I applied the fix but now we encounter a crash due to a check that doesn’t pass.

It’s in PCGMetadataDomain.cpp at line 820 in FPCGMetadataDomain::CopyAttribute.

The check is:

check(OriginalAttribute->GetMetadataDomain()->GetRoot() == GetRoot() || !bKeepParent);

Would it be safe to remove this check?

[Attachment Removed]

8

Huh that’s super weird, do you have some repro for this? I can’t see how this could happen.

[Attachment Removed]

9

Sorry it took me a while to get to it. This simple example triggers two checks [Image Removed]

The first check is at line 820 in FPCGMetadataDomain::CopyAttribute, PCGMetadataDomain.cpp

check(OriginalAttribute->GetMetadataDomain()->GetRoot() == GetRoot() || !bKeepParent);

The second one is at line 238, in FPCGMetadataAttribute::CopyInternal, PCGMetadataAttributeTpl.h

check(!bKeepParent || PCGMetadataHelpers::HasSameRoot(Metadata, InMetadata));

[Attachment Removed]

10

Ah crap that might be because parenting is not supported on Param Data. I’ll have a look thanks a lot!

[Attachment Removed]

11

I just tested in 5.8 and it doesn’t check, so maybe we changed some stuff in 5.8 on top of that!

I’ll re-sync 5.7 and apply my fix and check what could be done there.

[Attachment Removed]

12

Alright I found the difference. The check you got was changed in 5.8 to a checkSlow, which doesn’t check anymore x)

But the logic is flawed in our case if we do not support parenting, we should not keep the parent (like in Param Data).

So the full fix would be like this:

In FPCGMetadataDomain::CopyAttribute you should have this instead:

// Copy entries only if they come from the same data and they are on the same domain.
// We'll need to keep the parent too if we copy entries, but only if we support parenting (otherwise it makes no sense as there is no parent)
const bool bCopyEntries = bSameOrigin && SourceAttribute->GetMetadataDomain()->GetDomainID() == TargetMetadataDomain->GetDomainID();
const bool bKeepParent = bCopyEntries && TargetMetadata->MetadataDomainSupportsParenting(LocalDestinationAttribute.MetadataDomain);
const bool bSourceSupportsMultiEntries = SourceMetadata->MetadataDomainSupportsMultiEntries(LocalSourceAttribute.MetadataDomain);
const bool bTargetSupportsMultiEntries = TargetMetadata->MetadataDomainSupportsMultiEntries(LocalDestinationAttribute.MetadataDomain);
 
if (FPCGMetadataAttributeBase* NewAttr = TargetMetadataDomain->CopyAttribute(SourceAttribute, LocalDestinationAttribute.Name, /*bKeepParent=*/bKeepParent, /*bCopyEntries=*/bCopyEntries, /*bCopyValues=*/true))

Unfortunately this was shipped as is (the initial fix I sent you) in 5.8, so I’ll propose a hotfix in 5.8.1 if it is possible.

[Attachment Removed]

13

This last fix seems to have done the trick! Thanks again for looking into this!

[Attachment Removed]