holoviz-topics / imagen

ImaGen: Generic Python library for 0D, 1D and 2D pattern distributions
https://imagen.holoviz.org/
BSD 3-Clause "New" or "Revised" License
31 stars 16 forks source link

Small thickness lines disappear at some orientation #23

Closed JoelChavas closed 10 years ago

JoelChavas commented 10 years ago

Motivation behind the changes

By generating stimuli for illusory contour experiments, I have observed that, when the lines have a small thickness, they disappear at some orientation (particularly 0 and pi/2). See the test code below. The fact that it is orientation-dependent may be annoying. I propose in the pattern generator Line to check the thickness with respect to the pixel size. This change guarantees that, even with a thickness of 0, you will get a line After this change, I propose to get a little bit further, that is to guarantee that the line of the smallest width that you get is always 1 pixel. Again, in the current code, you get this feature at nearly all orientations except close to 0 and pi/2. To get this feature at all orientations, I propose in this pull request a change in patternfn.py/line.

Test code

import matplotlib.pyplot as plt import matplotlib.cm as cm from numpy import pi

import imagen from dataviews.sheetviews.boundingregion import BoundingBox

bounds=BoundingBox(radius = 6) xdensity = 20 ydensity = 20

fig = plt.figure()

def plot(image): im = plt.imshow(image,cmap=cm.Greys, interpolation='none', aspect='auto', extent=[-6,6,-6,6])

def one_line(orientation=pi/4): ima = imagen.Line(scale=-1,offset=1,orientation=orientation,thickness=0.03, authorize_zero_thickness=False,x=0., y=0., smoothing=0.0, xdensity=xdensity, ydensity=ydensity,bounds=bounds) return ima()

plt.ylim([0,1])

for i in xrange(1,7): ax=plt.subplot(2,3,i) angle=(i-1)/10. ax.set_title('angle='+str(angle)+'pi') plot(one_line(orientation=(angle*pi)))

plt.show()

Results of the test code

Initial behavior: at orientation 0 and pi/2, the lines disappear: line_disappear_ko

Intermediate behavior after the first correction: the minimal width is 2 pixels at 0 and pi/2, whereas it is 1 pixel at other orientations: minimal_line_wider_than_1_pixel_ko

Final desired behavior: minimal line width is one pixel at all orientations (the sixth panel has a line of width one, not visible here due to the resolution): minimal_thickness__one_pixel_ok

JoelChavas commented 10 years ago

OK, I have implemented your suggestions: print statement, rename test-->tests an suppress the README. Contents of the functions of tests made as one-liners. Note the I have kept here the SquareGrating modifications as they have been included in the main stream through the acceptance of the other pull request.

More important: I have added the parameter authorize_zero_thickness (True by default--> True means the classical behavior), and changed accordingly the code and the test.

The only suggestion not implemented: the hack 0.001*xpixelsize. I don't agree with the fact that this hack makes the change dependent on the density. I think on the contrary that it permits to render the expected behavior (minimal thickness of 1 pixel) independent on the density. However, I agree with you that it remains a hack. It is the best one I could find without having to touch patternfn/line. Tell me what you think about it.

jbednar commented 10 years ago

Can you tell me what you would change in patternfn/line, to see if it is better than the proposed 0.001 hack?

JoelChavas commented 10 years ago

I have not found yet a good solution, even using the patternfn/line. Particularly, by changing the value of 'x', I can't make run the tests (I have added a test with a change on the value of 'x' that is smaller than the pixelsize). So, even the hack is not good. I am still trying to find a solution...

ANyway, the solution that I proposed (well, the hack) is to solve the problem: line execatly between two pixel points. In this case, it has a thickness of 2 pixels in the current framework (that is the modifications put in Line), without the hack

JoelChavas commented 10 years ago

So, I removede this 0.001 hack. I have created a line_one_pixel similar to line in patternfn.py. The "solution" is to calculate two shifted lines (by half a pixel) and takes the one that is the smallest.

The question that remains: how to arrive to this in a cleaner way?

jbednar commented 10 years ago

Thanks; that's an interesting approach. I should mention that earlier when I said that the 0.001 hack would depend on the density, I should have said that it would depend on the precise position of the line relative to the pixel grid -- the lines disappear when they fall precisely betwen the centers of two pixels, and so adding a small offset can make it hit the centers of other pixels, but not reliably. So I'm glad you aren't proposing to do that anymore.

For line_one_pixel, first, it looks to me that all it's effectively doing is to change the first argument to the line() function, and so it seems like it can be reformulated so that it no longer includes the content of line(), but instead simply makes a new y array to feed into the unmodified line() function. I.e. you can have Line do return line(self.pattern_y if not p.enforce_minimal_thickness else self.minimal_y(),p.thickness,p.smoothing for some new method Line.minimal_y(). That way you wouldn't need any changes to patternfn.py. Then in the new method, it would seem cleaner to avoid the d and g lists, appending, etc., using a temporary helper function "count_negative_distances" on the two possible options. Anyway, it does seem like you could do something like what you propose, if you clean it up and if you document that when this option is enabled, the position of the line may be shifted by half a pixel to ensure that it lands on pixel centers.

JoelChavas commented 10 years ago

I have implemented your changes. Do they look fine to you?

jbednar commented 10 years ago

Looks much cleaner now, thanks!

jbednar commented 10 years ago

I cleaned up the docstring and method names, and it works well when I try it in the GUI. Should be all done now!