Skip to content

Update the pixel_shader usage of OnDiskBitmap #25

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

Merged
merged 2 commits into from
Feb 19, 2022

Conversation

lesamouraipourpre
Copy link
Contributor

OnDiskBitmap has had incompatible changes in 7.0.0-alpha.3 - Ref: adafruit/circuitpython#4823
This PR is part of a group of updates for this change - adafruit/circuitpython#4982

This PR updates the library to the combined usage for CP6 & CP7 with a TODO to remove the CP6 compatibility in the future. An example was added examples/turtle_bgpic_changeturtle.py that calls the methods that use OnDiskBitmap for testing.

It has been tested on a PyPortal with 6.3.0 and 7.0.0-alpha.4

Copy link
Contributor

@jepler jepler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! I did not test.

@FoamyGuy
Copy link
Contributor

Something seems odd in this library. I tested the new turtle_bgpic_changeturtle.py example using a PyPortal with both 6.3.0 and 7.0.0-alpha.4

On 6.3.0 using the currently released version of the library it outputs like this:
image

On 7.0.0-alpha.4 using the version from this PR it looks like this:
image

The blue color for the lines doesn't seem to be getting used properly, and the stamp in the center gets "filled in" by the lines instead of being drawn on top of like in the other version.

Nothing problematic really jumps out about the changes from this PR though so I'm a bit stumped as to what could be causing this difference at first glance.

I will try to take deeper look through the library code to see if I can figure out what is going on this weekend. I tested a few of the other examples (but not all) and didn't notice any issues with them so I am thinking perhaps this issues is related specifically to using changeturtle() to set an image as the turtle pen.

@dhalbert
Copy link
Contributor

@lesamouraipourpre @FoamyGuy This still seems to be unresolved. Any ideas?

@FoamyGuy
Copy link
Contributor

I never did get back into it.

I am taking a closer look today.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-tested this PR with PyPortal 7.2.0-rc.0 and 7.1.1

And I am not seeing the weird behavior from my second image above anymore. That seems to have been resolved in the interim in the core or somewhere other than this library.

The changes here look good to me.

By now I think we are okay to remove the CP 6 fallback and I think it would also be okay to remove the now-artificial limitation on things added to the addon_group. I'll make a follow up PR for those.

@FoamyGuy FoamyGuy merged commit cb3d4b5 into adafruit:main Feb 19, 2022
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 20, 2022
Updating https://github.com/adafruit/Adafruit_CircuitPython_GC_IOT_Core to 3.1.1 from 3.0.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_GC_IOT_Core#21 from FoamyGuy/fix_format
  > Merge pull request adafruit/Adafruit_CircuitPython_GC_IOT_Core#20 from geudrik/fix_ntp_requirement
  > Fixed readthedocs build
  > Post-patch cleanup
  > Consolidate Documentation sections of README
  > Consolidate Documentation sections of README

Updating https://github.com/adafruit/Adafruit_CircuitPython_turtle to 2.2.4 from 2.2.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_turtle#25 from lesamouraipourpre/ondiskbitmap-changes
  > Fixed readthedocs build
  > Consolidate Documentation sections of README
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.

4 participants