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

internal/shader: kage considers variable unused when it's used for assigning value to array #2848

Closed
1 of 11 tasks
imprity opened this issue Nov 18, 2023 · 5 comments
Closed
1 of 11 tasks

Comments

@imprity
Copy link

imprity commented Nov 18, 2023

Ebitengine Version

v2.6.2

Operating System

  • Windows
  • macOS
  • Linux
  • FreeBSD
  • OpenBSD
  • Android
  • iOS
  • Nintendo Switch
  • PlayStation 5
  • Xbox
  • Web Browsers

Go Version (go version)

go1.21.0

What steps will reproduce the problem?

When compling shader code, kage thinks that local variable is unused even though it's used for assigning value to array.

Here's the code for reproducing the error.

package main

import (
	"github.com/hajimehoshi/ebiten/v2"
	"log"
)

const (
	ScreenWidth  = 600
	ScreenHeight = 600
)

const ShaderCode = `
package main

func Fragment(position vec4, texCoord vec2, color vec4) vec4{
	var randomMatrix [5]float

	for x:=-2; x<=2; x++{
		//kage thinks that i is unused even though it's not
		i := x + 2 
		randomMatrix[i] = float(x)
	}

	var sum float = 0.0

	for x:=-2; x<=2; x++{
		//j is fine though
		j := x + 2
		sum += randomMatrix[j]
	}

	return vec4(0)
}
`

var Shader *ebiten.Shader

type Game struct {}

func (g *Game) Update() error {
	return nil
}

func (g *Game) Draw(screen *ebiten.Image) {
	shaderOp := &ebiten.DrawRectShaderOptions{}
	screen.DrawRectShader(ScreenWidth, ScreenHeight, Shader, shaderOp)
}

func (g *Game) Layout(outsideWidth, outsideHeight int) (int, int) {
	return ScreenWidth, ScreenHeight
}

func main() {
	var err error

	if Shader, err =  ebiten.NewShader([]byte(ShaderCode)); err != nil{
		log.Fatal(err)
	}

	ebiten.SetWindowSize(ScreenWidth, ScreenHeight)
	ebiten.SetWindowTitle("Cool Window")
	if err := ebiten.RunGame(&Game{}); err != nil {
		log.Fatal(err)
	}
}

At line 23, even though i is being used for indexing array, Kage thinks it's unused. And outputs this error :
local variable i is not used

What is the expected result?

I think it should be compiled. ❤️

What happens instead?

It refused to compile. 😢

Anything else you feel useful to add?

No response

@imprity
Copy link
Author

imprity commented Nov 19, 2023

I'm not sure if it's the fix And I haven't tested it yet since I'm very new to open source projects,

But I think the fix is changing one line

in internal\shader\expr.go

change this

	case *ast.IndexExpr:
		var stmts []shaderir.Stmt

		// Parse the index first
		exprs, _, ss, ok := cs.parseExpr(block, fname, e.Index, markLocalVariableUsed)

to

	case *ast.IndexExpr:
		var stmts []shaderir.Stmt

		// Parse the index first
		exprs, _, ss, ok := cs.parseExpr(block, fname, e.Index, true)

(it was at line 916 when I checked)

When checking left hand side, markLocalVariableUsed is set to false, but I think it should be always marked as used since values are used for indexing.

I might be wrong but I hope this helps.

@hajimehoshi hajimehoshi added this to the v2.7.0 milestone Nov 19, 2023
@hajimehoshi
Copy link
Owner

Thanks! I'll take a look.

@hajimehoshi
Copy link
Owner

But I think the fix is changing one line

You are correct, thank you!

Usually, I don't backport the fix for shaders to the stable branch 2.6. However, if you want it, I'm fine to do it.

@imprity
Copy link
Author

imprity commented Nov 19, 2023

I think it would be really cool if it's backproted to 2.6!

Thanks!

@hajimehoshi
Copy link
Owner

hajimehoshi commented Nov 19, 2023

Cherry-picked. I'll add a tag v2.6.3 few days later. If you are in hurry, use the commit hash directly:

go get github.com/hajimehoshi/ebiten/v2@2c967dd24e63b388479c71f2648418c14f7b94b7

@hajimehoshi hajimehoshi changed the title Kage: kage considers variable unused when it's used for assigning value to array internal/shader: kage considers variable unused when it's used for assigning value to array Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants