Issue №1070 opened by illume at 2019-05-26 11:20:04

Reading the pygame/pygame#1029 issue again... helpful would be some sort of Mask drawing functions.

Some ideas about a possible API...

Mask.to_surface() -> Surface with the set ones as white, the unset ones as black.

Mask.to_surface(surface, setcolor=(255,255,255,255), unsetcolor=None)
-> draws setcolor onto the surface where the mask is set to 1.

Mask.to_surface(surface, setcolor=(255,255,255,255), unsetcolor=(0,0,0,127))
Mask.to_surface(surface, (255,255,255,255), (0,0,0,127))
-> draws setcolor onto the surface where the mask is set to 1, 
    and unsetcolor where the mask is set to 0.

Mask.to_surface(surface, setsurface=surf, unsetsurface=None)
 -> draws on surface, using values from setsurface where the mask is set to 1.

Mask.to_surface(surface, setsurface=surf, unsetcolor=(255,255,255,127))
 -> draws on surface, using values from setsurface where the mask is set to 1.
    and unsetcolor where the mask is set to 0.

Mask.to_surface(surface=None, setsurface=surf)
Mask.to_surface(setsurface=surf)
->creates a new surface because surface=None, based on the setsurface settings.

Mask.to_surface(offset=(10, 10)) -> Surface

This would allow drawing on an existing surface, as well as making it easy to create a new surface based on the mask.

to_surface is used here because it is the opposite of from_surface.

Related Docs: https://www.pygame.org/docs/ref/mask.html# pygame.mask.Mask.to_surface

Comments

# # illume commented at 2019-05-26 11:26:20

Another approach to think about is to make a Mask be an acceptable Surface. But this is a LOT more work. The idea being that Surface.blit(mask, ...) and pygame.draw.line(mask, ...) would somehow work. Mask could have to have setcolor, and unsetcolor attributes perhaps (so when they are blit onto a surface we know what to draw).

# # DeluxeYang commented at 2019-05-27 06:44:09

this would be really helpful

# # charlesej commented at 2019-05-27 13:20:05

Work items for mask.to_surface().

• support for surface, setcolor and unsetcolor kwargs

  • code
  • tests
  • documentation

• support for setsurface and unsetsurface kwargs

  • code
  • tests
  • documentation

• support for dest kwarg

  • code
  • tests
  • documentation

• support for area kwarg

  • code
  • tests
  • documentation

• support for creating a new surface, i.e. when surface=None

  • code
  • tests
  • documentation

• change kwargs: setcolor to oncolor, unsetcolor to offcolor, setsurface to onsurface, and unsetsurface to offsurface

  • code
  • documentation

# # illume commented at 2019-05-27 13:31:10

Thanks for the checklist @charlesej

I left blending and all that type of stuff out of the API, because it's probably quite a lot of work. The functions here would be like a standard fill/copy of colors.

# # charlesej commented at 2019-05-27 13:35:16

@illume What are your thoughts with regards to the offset parameter? Do you want to use the offset to signify where in the mask to start drawing or where on the surface to start drawing?

#  Start drawing the mask's position (0, 0) at position (5, 10) on the given surface.
Mask.to_surface(surface, offset=(5, 10), unsetcolor=None)

And/or?

#  Start drawing the mask's position (5, 10) at position (0, 0) on the created surface.
Mask.to_surface(offset=(5, 10))

# # illume commented at 2019-05-27 13:48:10

Good question.

For offset, I was looking at the overlap method of Mask.
But maybe we should copy the Surface.blit arguments dest, and area instead?

• area rectangle represents a smaller portion of the source mask to draw.
• dest rectangle is where the drawing on the destination surface should happen. Useful if surface is larger than than the mask, or they just want to draw somewhere else on the surface.

# # illume commented at 2019-07-06 14:53:54

Thanks for the setsurface/unsetsurface support @charlesej . I'm really looking forward to using that.

# # charlesej commented at 2019-07-06 17:59:00

You are welcome.

There is one thing that I didn't implement with regards to the setsurface parameter from your example code.

Mask.to_surface(surface=None, setsurface=surf)
Mask.to_surface(setsurface=surf)
->creates a new surface because surface=None, based on the setsurface settings.

The setsurface settings are currently not used when creating a new surface. The new surface is always created with flags=SRCALPHA and depth=32. I felt it would be better if the setsurface and unsetsurface parameters were only used for colors. The user can always pass a surface= parameter if they wanted something other than the default surface creation.

Do you still want the setsurface settings to be used when creating a new surface? Also if no setsurface is provided should the settings from unsetsurface be used?

# # illume commented at 2019-07-06 19:00:55

I agree that people can pass in a surface if they want to specify different flags. Using setsurface for this would be a bit confusing IMHO.

# # illume commented at 2019-07-06 19:01:37

Just now, reading this code by itself isn't as clear as it could be...

Mask.to_surface(setsurface=surf)

setsurface=surf sort of looks like we are setting the surface.

Maybe we need to rethink the wording here?

# # charlesej commented at 2019-07-07 15:47:17

Agreed.

Instead of the setsurface / unsetsurface keywords how about:

Mask.to_surface(setfrom=surface1)
Mask.to_surface(unsetfrom=surface2)
Mask.to_surface(setfrom=surface1, unsetfrom=surface2)

OR

Mask.to_surface(setfromsurf=surface1)
Mask.to_surface(unsetfromsurf=surface2)
Mask.to_surface(setfromsurf=surface1, unsetfromsurf=surface2)

# # illume commented at 2019-07-11 11:34:34

How about these?

Mask.to_surface(offcolor=Color('blue'))
Mask.to_surface(oncolor=Color('red'))

Mask.to_surface(offcolors=surface1)
Mask.to_surface(oncolors=surface1)

Mask.to_surface(surface, offcolors=surface1)
Mask.to_surface(surface, oncolors=surface1)

offcolors, oncolors is better than offsurface onsurface I think?

Mask.to_surface(offcolor=Color('blue'))
Mask.to_surface(oncolor=Color('red'))

Mask.to_surface(offsurface=surface1)
Mask.to_surface(onsurface=surface1)

Mask.to_surface(surface, offsurface=surface1)
Mask.to_surface(surface, onsurface=surface1)

# # charlesej commented at 2019-07-11 15:31:46

Using oncolor and oncolors seems like it might cause some confusion. So for clarity I would favor using oncolor / offcolor and onsurface / offsurface.

# # illume commented at 2019-07-15 07:56:32

Cool.

Thinking about making it blit() api compatible... somehow.

mask.oncolor = 'black'
mask.offcolor = 'white'
mask.onsurface = s
mask.offsurface = s2
mask.blit(surface, (0, 0))

Maybe default to oncolor == 'white' and everything else off would be ok default.

mask.oncolor = 'red'
mask.blit(surface, (0, 0))

# # charlesej commented at 2019-07-22 14:43:44

Were you thinking of adding this in addition to the to_surface() method or replacing it? My thoughts would be to get the rest of the to_surface() functionality (dest and area parameters) working and tested before attempting the blit interface.

# # illume commented at 2019-07-22 14:48:54

I think keeping to_surface is fine and good. Yes, finishing it off first is a better idea.

# # robertpfeiffer commented at 2019-07-22 16:22:41

So how many steps do I have to take when I want to blit from surf A to surf B, but only the pixels from surf A where the mask is set?

# # illume commented at 2019-07-22 16:59:40

mask.to_surface(surfaceB, onsurface=surfaceB)