Skip to content

Fix PositionScale Scale value passed to shader#28

Open
jamierobertson1 wants to merge 1 commit intovsg-dev:masterfrom
jamierobertson1:positionscale_fix
Open

Fix PositionScale Scale value passed to shader#28
jamierobertson1 wants to merge 1 commit intovsg-dev:masterfrom
jamierobertson1:positionscale_fix

Conversation

@jamierobertson1
Copy link
Copy Markdown
Contributor

I noticed there is small rendering error when points are scaled in the shader.

Currently the points positions are moved and scaled in the shader as follows:

    #ifdef VSG_POSITION_SCALE
    vec4 vertex = vec4(vsg_PositionScale.xyz + vsg_Vertex * vsg_PositionScale.w, 1.0);
    #else 

Where the vsg_Vertex is an 0-1 float calculate from one of Vulkans UNORM integer types depending on the bits used in the VSGPoints::Settings.

Currently VSGPoints is sending the bricksize as the scaling value.

double brickSize = brickPrecision * pow(2.0, static_cast<double>(settings.bits));

When a brick for a position are calculated, it uses the full pow(2.0,nbits) bit range for a brick (i.e. divides the original position by this), and the remainder makes up the position. As an example (ignoring precision), for 8 bits, each brick will be 256 units in length, with the possible remainder being in the 0-255 range as per image below:

image

In the shader assuming , a value of 255 (VK_FORMAT_R8G8B8_UNORM) will be interpreted as 1.0:
(from vulkan conversion docs):
image
The 255 value will then be scaled by the full brick size vsg_PositionScale.w (i.e. the full 256 size) This is incorrect as it should be one precision increment less (as per diagram above), so currently the points are scaled a bit too much on screen.

Simple fix is to subtract one precision increment from the positionScale, scale value in the Brick::createRendering function. The rest of the positionScale stuff calculations seem good, it's literally just the bit used by the shader which needs to be updated.

Change:

vsg::vec4 positionScale(position.x, position.y, position.z, brickSize);
return createRendering(settings, positionScale, pointSize);

to:

vsg::vec4 positionScale(position.x, position.y, position.z, brickSize - brickPrecision);
return createRendering(settings, positionScale, pointSize); 

I verified this by drawing a wireframe box of known size, and then adding VSGPoint points at the corners.

Currently:

image

After fix:

image

For testing I just modified the VSGPoints application, to draw the box and points if no input file is supplied:

    if (group->children.empty())
    {
        // add box
        vsg::StateInfo stateInfo;
        stateInfo.wireframe = true;
        auto builder = vsg::Builder::create();
        builder->options = options;

        vsg::GeometryInfo info;
        info.cullNode = true;
        info.dx.set(2.f, 0.0f, 0.0f);
        info.dy.set(0.0f, 2.0f, 0.0f);
        info.dz.set(0.0f, 0.0f, 2.0f);
        auto box = builder->createBox(info, stateInfo);
        group->addChild(box);

        // add vsgpoints for corners
        settings = vsgPoints::Settings::create();
        settings->createType = vsgPoints::CREATE_FLAT;
        settings->precision = 0.001;
        settings->bits = 8;
        auto bricks = vsgPoints::Bricks::create(settings);

        // checked fixed coordinates
        bricks->add(vsg::dvec3(-1.0, -1.0, -1.0), vsg::ubvec4(0, 255, 0, 255));
        bricks->add(vsg::dvec3(-1.0, 1.0, -1.0), vsg::ubvec4(0, 255, 0, 255));
        bricks->add(vsg::dvec3(-1.0, -1.0, 1.0), vsg::ubvec4(0, 255, 0, 255));
        bricks->add(vsg::dvec3(-1.0, 1.0, 1.0), vsg::ubvec4(0, 255, 0, 255));
        bricks->add(vsg::dvec3(1.0, -1.0, 1.0), vsg::ubvec4(0, 255, 0, 255));
        bricks->add(vsg::dvec3(1.0, 1.0, 1.0), vsg::ubvec4(0, 255, 0, 255));
        bricks->add(vsg::dvec3(1.0, -1.0, -1.0), vsg::ubvec4(0, 255, 0, 255));
        bricks->add(vsg::dvec3(1.0, 1.0, -1.0), vsg::ubvec4(0, 255, 0, 255));
        group->addChild(vsgPoints::createSceneGraph(bricks, settings));
    }

Tested on windows

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.

1 participant