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

Draw order issue #318

Closed
2 of 5 tasks
BenDol opened this issue Aug 11, 2022 · 5 comments
Closed
2 of 5 tasks

Draw order issue #318

BenDol opened this issue Aug 11, 2022 · 5 comments
Labels
Priority: Medium This issue may be impactful and needs some attention. Status: Pending Test This PR or Issue requires more testing Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.

Comments

@BenDol
Copy link
Collaborator

BenDol commented Aug 11, 2022

Priority

Medium

Area

  • Data
  • Source
  • Docker
  • Other

What happened?

Seems there's something wrong with the draw ordering:

titanclient_gB9a1IJzxw

On regular otclient

titanclient_OI7vqayp2b

I'm on the latest commit as of the time of this posting.

I'll be investigating too, but figured I'd report this issue.

What OS are you seeing the problem on?

Windows

Code of Conduct

  • I agree to follow this project's Code of Conduct
@BenDol BenDol added the Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Aug 11, 2022
@github-actions github-actions bot added Priority: Medium This issue may be impactful and needs some attention. Status: Pending Test This PR or Issue requires more testing labels Aug 11, 2022
@BenDol
Copy link
Collaborator Author

BenDol commented Aug 11, 2022

Investigation Notes

The wall item:
image
Draws here in tile.cpp#draw:

if (m_countFlag.hasBottomItem) {
    for (const auto& item : m_things) {
        if (!item->isOnBottom()) continue;
        drawThing(item, dest - m_drawElevation * scaleFactor, scaleFactor, true, flags, lightView);
    }
}

The Fire Item:
image
Draws here in tile.cpp#draw:

if (m_countFlag.hasCommonItem) {
    for (const auto& item : m_things | std::views::reverse) {
        if (!item->isCommon()) continue;
        drawThing(item, dest - m_drawElevation * scaleFactor, scaleFactor, true, flags, lightView);
    }
}

Though likely an issue in the DrawPool ?

EDIT: I think the issue is that we're drawing all the items on each tile, rather than drawing on tiles by "layer"
Shouldn't we draw bottomItem's on all tiles then draw the commonItems on all tiles after?

@BenDol
Copy link
Collaborator Author

BenDol commented Aug 11, 2022

This likely isn't the greatest solution, but if I copy paste the tiles loop and call a drawBottom which draws ground and bottom items and call this on all the tiles before we draw everything else it resolves the issue.

void Tile::drawBottom(const Point& dest, const MapPosInfo& mapRect, float scaleFactor, int flags, bool isCovered, LightView* lightView)
{
    m_drawElevation = 0;
    m_lastDrawDest = dest;

    for (const auto& thing : m_things) {
        if (!thing->isGround() && !thing->isGroundBorder())
            break;

        auto id = thing->getId();
        drawThing(thing, dest - m_drawElevation * scaleFactor, scaleFactor, true, flags, lightView);
    }

    if (m_countFlag.hasBottomItem) {
        for (const auto& item : m_things) {
            if (!item->isOnBottom()) continue;
            auto id = item->getId();
            drawThing(item, dest - m_drawElevation * scaleFactor, scaleFactor, true, flags, lightView);
        }
    }
}

(This isn't optimal, just copy pasting the original loop and add it prior to the draw tile loop)

for (const auto& tile : map.tiles) {
    uint32 tileFlags = flags;

    if (!m_drawViewportEdge && !tile->canRender(tileFlags, cameraPosition, m_viewport, lightView))
        continue;

    bool isCovered = false;
    if (tile->hasCreature()) {
        isCovered = tile->isCovered(m_cachedFirstVisibleFloor);
    }

    if (alwaysTransparent) {
        const bool inRange = tile->getPosition().isInRange(_camera, TRANSPARENT_FLOOR_VIEW_RANGE, TRANSPARENT_FLOOR_VIEW_RANGE, true);
        isCovered = isCovered && !inRange;

        g_drawPool.setOpacity(inRange ? .16 : .7);
    }

    tile->drawBottom(transformPositionTo2D(tile->getPosition(), cameraPosition), m_posInfo, m_scaleFactor, tileFlags, isCovered, lightView);
                
    if (alwaysTransparent)
        g_drawPool.resetOpacity();
}

Just demoing how I resolved it so far.

@BenDol
Copy link
Collaborator Author

BenDol commented Aug 11, 2022

Replaced with a lambda (until I find a better solution), this works and doesn't have a massive FPS impact.

drawTiles(map, cameraPosition, alwaysTransparent, flags, lightView, [&](const TilePtr& tile, uint32 tileFlags, bool isCovered) {
    tile->drawBottom(transformPositionTo2D(tile->getPosition(), cameraPosition), m_posInfo, m_scaleFactor, tileFlags, isCovered, lightView);
});

drawTiles(map, cameraPosition, alwaysTransparent, flags, lightView, [&](const TilePtr& tile, uint32 tileFlags, bool isCovered) {
    tile->draw(transformPositionTo2D(tile->getPosition(), cameraPosition), m_posInfo, m_scaleFactor, tileFlags, isCovered, lightView);
});

Look forward to a more optimized solution if there is one.

EDIT: This actually has some problems right now with other items drawing underneath things like railings, etc

image

@mehah
Copy link
Owner

mehah commented Aug 11, 2022

try this:
on the line:

void Thing::generateBuffer()

change to:

void Thing::generateBuffer()
{
    m_drawBuffer = nullptr;

    DrawPool::DrawOrder order = DrawPool::DrawOrder::NONE;
    if (isSingleGround())
        order = DrawPool::DrawOrder::FIRST;
    else if (isGroundBorder())
        order = DrawPool::DrawOrder::SECOND;
    else if ((isCommon() || isOnBottom()) && isSingleDimension() && !hasDisplacement() && isNotMoveable())
        order = DrawPool::DrawOrder::THIRD;
    else if (isTopGround() || g_app.isDrawingEffectsOnTop() && isEffect())
        order = DrawPool::DrawOrder::FOURTH;
    else if (isMissile())
        order = DrawPool::DrawOrder::FIFTH;

    if (order != DrawPool::DrawOrder::NONE)
        m_drawBuffer = std::make_shared<DrawBuffer>(order);
}

for you to understand, i put it to check if the common item has offset, because if it does, it won't try to group with a similar item that was rendered before it.

@BenDol
Copy link
Collaborator Author

BenDol commented Aug 11, 2022

Nice, yep that solves it. Thanks!

@mehah mehah closed this as completed in d40a411 Aug 11, 2022
dudantas pushed a commit that referenced this issue Nov 17, 2023
Description:
Update tibia protocol to 12.91, including the following features:
• Timer on rings/amulets.
• Charge number on rings/amulets.
• Npc fix. (Sell/Buy more than 100 items at once)
• Two new depot boxes

Fixes:
• #318#386

Client 12.91 download link: https://github.com/dudantas/tibia-client/releases/tag/12.91.12329
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium This issue may be impactful and needs some attention. Status: Pending Test This PR or Issue requires more testing Type: Bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
None yet
Development

No branches or pull requests

2 participants