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

STARK: Implement the shadow #1483

Merged
merged 12 commits into from Aug 6, 2018

Conversation

Projects
None yet
3 participants
@DouglasLiuGamer
Copy link
Collaborator

DouglasLiuGamer commented Jul 25, 2018

This PR is corresponding to implementing the characters' shadows in TLJ.

@DouglasLiuGamer DouglasLiuGamer requested a review from bgK Jul 25, 2018

@DouglasLiuGamer

This comment has been minimized.

Copy link
Collaborator Author

DouglasLiuGamer commented Jul 25, 2018

A screenshot of the current version. The shadow is directly cast from above.

shadow

glEnable(GL_BLEND);
glEnable(GL_STENCIL_TEST);

_shadowShader->enableVertexAttribute("position1", _faceVBO, 3, GL_FLOAT, GL_FALSE, 14 * sizeof(float), 0);

This comment has been minimized.

@Botje

Botje Jul 26, 2018

Member

Although you're just emulating code that is already written, I would like to remark two things:

  1. Vertex attributes should only be set once, when the corresponding VBO object is created, rather than for every render operation.
  2. Both the stride (second to last parameter) and the offset (last parameter) should be defined in terms of multiples of floats.
@bgK
Copy link
Member

bgK left a comment

This looks pretty good already, well done.

@@ -115,6 +117,9 @@ class RenderEntry {
float _direction3D;
float _sortKey;
bool _clickable;

bool _castsShadow;

This comment has been minimized.

@bgK

bgK Jul 28, 2018

Member

_castsShadow should be a property of VisualActor.

@@ -115,6 +117,9 @@ class RenderEntry {
float _direction3D;
float _sortKey;
bool _clickable;

bool _castsShadow;
uint32 _maxShadowLength;

This comment has been minimized.

@bgK

bgK Jul 28, 2018

Member

_maxShadowLength should be a property of Scene.


if (castsShadow && StarkSettings->getBoolSetting(Settings::kShadow)) {
glEnable(GL_BLEND);
glEnable(GL_STENCIL_TEST);

This comment has been minimized.

@bgK

bgK Jul 28, 2018

Member

I'm not under the impression the stencil buffer is actually used.

This comment has been minimized.

@Botje

Botje Jul 30, 2018

Member

See the glStencilFunc and glStencilOp calls in set3DMode. The first makes it so that every touched pixel ends with a stencil value of 1 (or more if overdraw occurs), while the second discards pixels that have a non-zero stencil value while drawing shadows. This prevents the shadows from overlapping and creates a uniform shadow shape.

This comment has been minimized.

@DouglasLiuGamer

DouglasLiuGamer Jul 30, 2018

Author Collaborator

Yup~

OUTPUT

void main() {
outColor = vec4(0.0, 0.0, 0.0, 0.6);

This comment has been minimized.

@bgK

bgK Jul 28, 2018

Member

Isn't the shadow a bit too dark ?


//Transform the direction to the model space and pass to the shader
sumDirection = worldToModelRot * sumDirection;
_shadowShader->setUniform("lightDirection", sumDirection);

This comment has been minimized.

@DouglasLiuGamer

DouglasLiuGamer Aug 3, 2018

Author Collaborator

Rather than computing a matrix for casting shadows for vertices in general, I think it is a lot easier and intuitive to transform the light direction to the model space and then do the projection in the shader.

@DouglasLiuGamer

This comment has been minimized.

Copy link
Collaborator Author

DouglasLiuGamer commented Aug 3, 2018

With the tremendous help from @bgK who helped me to look at the disassembly, now the shadow is computed based on the actual lights. Some screenshots of the current version here, I temporarily rendered the shadow in solid green for better illustration.

longshadow1

longshadow2

In location 39 00 the shadow will get long and change direction when April moves.

return false;
}

return getPointLightContribution(light, actorPosition, direction);

This comment has been minimized.

@bgK

bgK Aug 3, 2018

Member

cone should affect direction.

}
bool OpenGLSActorRenderer::getSpotLightContribution(LightEntry *light,
const Math::Vector3d &actorPosition, Math::Vector3d &direction) {
Math::Vector3d LightToActor = actorPosition - light->position;

This comment has been minimized.

@bgK

bgK Aug 3, 2018

Member

That variable name should be lowerCamelCase.

@DouglasLiuGamer DouglasLiuGamer force-pushed the DouglasLiuGamer:branch-shadow branch from c43d9bd to 80fa29b Aug 4, 2018

@DouglasLiuGamer

This comment has been minimized.

Copy link
Collaborator Author

DouglasLiuGamer commented Aug 4, 2018

@Botje About the two remarks you made, I am not sure about the first one. Based on the current logic in the rendering code, the same shader is used for different models, which has there own VBOs. Therefore it should be natural to bind the VBO to vertex attributes everytime the model is drawn, at least this is what I think about it.

Well, I am new to OpenGL, so I may be wrong.

@DouglasLiuGamer DouglasLiuGamer force-pushed the DouglasLiuGamer:branch-shadow branch from 80fa29b to 801fc74 Aug 4, 2018

@bgK
Copy link
Member

bgK left a comment

This is looking pretty good!

However, I noticed the shadow direction is sometimes changing abruptly, while it does not happen in the original. Maybe the calculation is sometimes slightly incorrect?

https://youtu.be/kC_hfzMF_DQ

Resources::Anim *anim = getAnim();
if (anim && anim->getSubType() == Anim::kAnimSkeleton) {
Resources::AnimSkeleton *animSkeleton = Resources::Object::cast<Resources::AnimSkeleton>(anim);
actor->setCastShadow(animSkeleton->castsShadow());

This comment has been minimized.

@bgK

bgK Aug 4, 2018

Member

This should be done in AnimSkeleton::applyToItem, removing the need for AnimSkeleton::castsShadow.

@DouglasLiuGamer

This comment has been minimized.

Copy link
Collaborator Author

DouglasLiuGamer commented Aug 5, 2018

@bgK I can't reproduce the case in your video. The shadow on my computer there changes seamlessly. Did you use the latest version where the cone of the spotlight affects the direction?

@DouglasLiuGamer DouglasLiuGamer force-pushed the DouglasLiuGamer:branch-shadow branch from 4b48402 to 5f8a8fc Aug 5, 2018

@bgK
Copy link
Member

bgK left a comment

I can't reproduce the case in your video. The shadow on my computer there changes seamlessly. Did you use the latest version where the cone of the spotlight affects the direction?

I can't reproduce it anymore. I was indeed probably using an old version.

@@ -453,6 +453,9 @@ void AnimSkeleton::applyToItem(Item *item) {
_visual->setTextureFacial(nullptr);
_visual->setAnim(_seletonAnim);
_visual->setTime(_currentTime);

VisualActor *actor = _visual->get<VisualActor>();

This comment has been minimized.

@bgK

bgK Aug 5, 2018

Member

This can just be _visual->setCastShadow(_castsShadow);

@DouglasLiuGamer

This comment has been minimized.

Copy link
Collaborator Author

DouglasLiuGamer commented Aug 6, 2018

I felt so stupid for my last commit
@bgK If there is nothing more, I think we can do the merging.

@bgK bgK merged commit 8aaa260 into residualvm:master Aug 6, 2018

1 check passed

default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.