Douple pushing to clipElementStack results in Clay_PointerOver not working when pointer is indeed over
I found this bug where when you have a ClayElement that has both clip and floating behaviour and then after it in the UI tree you have some other elements, those other elements might not be inside the clay_context->pointerOverIds array used to check hoverability by Clay_PointerOver functions.
There is the following code inside Clay__ConfigureOpenElementPtr function:
if (declaration->floating.attachTo != CLAY_ATTACH_TO_NONE) {
/* some code was here, removed it for clarity*/
Clay__int32_tArray_Add(&context->openClipElementStack, clipElementId);
/* some code was here, removed it for clarity*/
}The code above adds id to clip element stack.
After the code above there is another if, here is a code snippet:
if (declaration->clip.horizontal || declaration->clip.vertical) {
// DD: I added this
if (!(declaration->floating.attachTo != CLAY_ATTACH_TO_NONE && (declaration->clip.horizontal || declaration->clip.vertical)))
{
Clay__int32_tArray_Add(&context->openClipElementStack, (int)openLayoutElement->id);
}
}The code above also adds id to the same stack.
Then inside Clay_SetPointerState that is done before Clay_BeginLayout is called this code is executed:
if ((Clay__PointIsInsideRect(position, elementBox)) && (clipElementId == 0 || (Clay__PointIsInsideRect(position, clipItem->boundingBox)) |context->externalScrollHandlingEnabled)) {
if (!skipTree) {
if (mapItem->onHoverFunction) {
mapItem->onHoverFunction(mapItem->elementId, context->pointerInfo, mapItem->hoverFunctionUserData);
}
Clay_ElementIdArray_Add(&context->pointerOverIds, mapItem->elementId);
}
found = true;
}In the code above this code only works if there is no clip element right now or the box you want to hover is also in the non-clipped zone. The logic there is correct.
But, back to the issue that I stated. If you have a ClayElement that is both floating and clipped you end up having Clay push its id twice on the same stack (openClipElementStack). That means that elements that are declared after this clip-floating container (but not as its children) if decide to use Clay_PointerOver for hover logic would not get valid hover data relative to what the UI visually represents since those elements won't be found in the context->pointerOverIds array because there was a double push to the context->openClipElementStack some time before.
I got this fixed by adding 3 lines of code in Clay__ConfigureOpenElementPtr function:
if (declaration->clip.horizontal || declaration->clip.vertical) {
/**/ // DD: I added this
/**/if (!(declaration->floating.attachTo != CLAY_ATTACH_TO_NONE && (declaration->clip.horizontal || declaration->clip.vertical)))
/**/{
/**/ Clay__int32_tArray_Add(&context->openClipElementStack, (int)openLayoutElement->id);
/**/}
// Retrieve or create cached data to track scroll position across frames
Clay__ScrollContainerDataInternal *scrollOffset = CLAY__NULL;
for (int32_t i = 0; i < context->scrollContainerDatas.length; i++) {
Clay__ScrollContainerDataInternal *mapping = Clay__ScrollContainerDataInternalArray_Get(&context->scrollContainerDatas, i);
if (openLayoutElement->id == mapping->elementId) {
scrollOffset = mapping;
scrollOffset->layoutElement = openLayoutElement;
scrollOffset->openThisFrame = true;
}
}
if (!scrollOffset) {
scrollOffset = Clay__ScrollContainerDataInternalArray_Add(&context->scrollContainerDatas, CLAY__INIT(Clay__ScrollContainerDataInternal){.layoutElement = openLayoutElement, .scrollOrigin = {-1,-1}, .elementId = openLayoutElement->id, .openThisFrame = true});
}
if (context->externalScrollHandlingEnabled) {
scrollOffset->scrollPosition = Clay__QueryScrollOffset(scrollOffset->elementId, context->queryScrollOffsetUserData);
}
}This is my first GitHub issue, so if I didn't explain myself well enough, sorry.
Source: nicbarker/clay