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

samples/simple-sphere.rb crashes on 64 bit systems #9

Closed
ccoupe opened this Issue Dec 23, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@ccoupe
Contributor

ccoupe commented Dec 23, 2014

If you turn the blur down to 25 from 30, it doesn't.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Dec 23, 2014

Contributor

The Shoes manual says nothing about the blur and some other transforms.

Contributor

ccoupe commented Dec 23, 2014

The Shoes manual says nothing about the blur and some other transforms.

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Dec 23, 2014

Collaborator

Could not duplicate the issue on Windows 8.1, 64 bit, Shoes 3.2.19. Even changing any or all blur to large values (over 100) do not cause Shoes to crash.

Collaborator

BackOrder commented Dec 23, 2014

Could not duplicate the issue on Windows 8.1, 64 bit, Shoes 3.2.19. Even changing any or all blur to large values (over 100) do not cause Shoes to crash.

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Dec 23, 2014

Collaborator

Duplicated the issue on MacOS X 10.6.8, Shoes 3.2.19 (my own build). Also confirming changing the value of blur at line 7 from 30 to 25 prevents Shoes from crashing. Console.app provides a backtrace from Ruby.

Collaborator

BackOrder commented Dec 23, 2014

Duplicated the issue on MacOS X 10.6.8, Shoes 3.2.19 (my own build). Also confirming changing the value of blur at line 7 from 30 to 25 prevents Shoes from crashing. Console.app provides a backtrace from Ruby.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Dec 24, 2014

Contributor

@BackOrder - the Shoes/Ruby for Windows is 32 bit.

Contributor

ccoupe commented Dec 24, 2014

@BackOrder - the Shoes/Ruby for Windows is 32 bit.

@ccoupe ccoupe added OSX Linux labels Dec 24, 2014

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Mar 11, 2015

Contributor

changing box_blur method in effects.c (adding a cast) works for me
https://github.com/Shoes3/shoes3/blob/master/shoes/effects.c#L71

      out[l2] = run[sums[0]];                        
      out[l2 + 1] = run[sums[1]];                    
      out[l2 + 2] = run[sums[2]];                    
      out[l2 + 3] = run[sums[3]]; 

to

      out[l2] = run[(int)sums[0]];                        
      out[l2 + 1] = run[(int)sums[1]];                    
      out[l2 + 2] = run[(int)sums[2]];                    
      out[l2 + 3] = run[(int)sums[3]]; 

might be related to some changes from 32bits to 64bits OS and the "value" of unsigned char ...
is casting to int the best option here ?

Contributor

passenger94 commented Mar 11, 2015

changing box_blur method in effects.c (adding a cast) works for me
https://github.com/Shoes3/shoes3/blob/master/shoes/effects.c#L71

      out[l2] = run[sums[0]];                        
      out[l2 + 1] = run[sums[1]];                    
      out[l2 + 2] = run[sums[2]];                    
      out[l2 + 3] = run[sums[3]]; 

to

      out[l2] = run[(int)sums[0]];                        
      out[l2 + 1] = run[(int)sums[1]];                    
      out[l2 + 2] = run[(int)sums[2]];                    
      out[l2 + 3] = run[(int)sums[3]]; 

might be related to some changes from 32bits to 64bits OS and the "value" of unsigned char ...
is casting to int the best option here ?

@ccoupe ccoupe added this to the 3.2.22 milestone Mar 11, 2015

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Mar 11, 2015

Contributor

@passenger94, that is wonderful! It works for my 64 linux. Please commit the change .

Contributor

ccoupe commented Mar 11, 2015

@passenger94, that is wonderful! It works for my 64 linux. Please commit the change .

@BackOrder

This comment has been minimized.

Show comment
Hide comment
@BackOrder

BackOrder Mar 11, 2015

Collaborator

Is this confirmed on MacOS X? If so, let's close the issue.

Collaborator

BackOrder commented Mar 11, 2015

Is this confirmed on MacOS X? If so, let's close the issue.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Mar 11, 2015

Contributor

Fixed on OSX as well.

Contributor

ccoupe commented Mar 11, 2015

Fixed on OSX as well.

@ccoupe ccoupe closed this Mar 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment