Skip to content

Cpu buffer#1538

Closed
thomasdeneux wants to merge 3 commits intovispy:masterfrom
thomasdeneux:CPU_buffer
Closed

Cpu buffer#1538
thomasdeneux wants to merge 3 commits intovispy:masterfrom
thomasdeneux:CPU_buffer

Conversation

@thomasdeneux
Copy link
Copy Markdown
Contributor

Following issue #1505, i implemented an optional local CPU buffer in the Buffer class. By default it is not used, but it is used by Collections so that one can set individual fields of the VertexBuffer.

thomas bureau added 3 commits October 15, 2018 18:06
- stopped in the middle
- probably CPU buffer should be added to child DataBuffer rather than to
  Buffer, especially because DataBuffer.__init__ does not call
  Buffer.__init__
@thomasdeneux
Copy link
Copy Markdown
Contributor Author

Hello, this pull request was not accepted, was it? Note that after implementing this "local CPU buffer" feature, i encountered more problems with collections, so i did not continue in that direction and now i am only writing my own shaders. So even myself i am not using this feature. Should we drop it or make the effort to go until acceptance of the PR because this might be useful in the future?

@djhoese
Copy link
Copy Markdown
Member

djhoese commented Mar 22, 2019

What kind of issues did you run in to with collections? If this gets us closer to more glumpy-like collections that are more flexible then I'm all for getting it merged.

Maybe you could merge this with current master and we'll see if the tests pass now? I have other changes (like CPUData -> cpu_data) that I'd like, but getting tests added and getting them passing is more important.

@thomasdeneux
Copy link
Copy Markdown
Contributor Author

If i try to do a new pull request i get a button "view pull request" that sends me back here, and no button to create a new pull request. So i assume that i must first close this current PR. I try it.

@djhoese
Copy link
Copy Markdown
Member

djhoese commented Mar 22, 2019

You can merge master in to your branch and push it to this same existing branch and the PR will update.

@thomasdeneux thomasdeneux mentioned this pull request Mar 22, 2019
@thomasdeneux
Copy link
Copy Markdown
Contributor Author

oh too late, i have already issued the new pull request...

@thomasdeneux
Copy link
Copy Markdown
Contributor Author

did not pass the tests again... and i have difficulty reading the test reports to determine where is the problem

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.

2 participants