Skip to content
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

Glide loses the initialized Registry #2885

Closed
wdkcjnwdwkldn opened this issue Feb 8, 2018 · 4 comments
Closed

Glide loses the initialized Registry #2885

wdkcjnwdwkldn opened this issue Feb 8, 2018 · 4 comments
Labels

Comments

@wdkcjnwdwkldn
Copy link

wdkcjnwdwkldn commented Feb 8, 2018

Glide version: 4.6.1

Issue details:
I configure Glide with GlideBuilder. WITHOUT GlideModule.
But MyModelLoaderFactory not working, because Glide loses the initialized Registry.

MyModelLoaderFactory not working:

val glide = GlideBuilder()
    ...
    .build(context)
glide.registry.append(MyModel::class.java, InputStream::class.java, MyModelLoaderFactory())
return glide

MyModelLoaderFactory works (deprecated):

val glide = GlideBuilder()
    ...
    .build(context)
Glide.init(glide) // deprecated
glide.registry.append(MyModel::class.java, InputStream::class.java, MyModelLoaderFactory())
return glide

GlideBuilder must be returns initialized Glide.

@sjudd
Copy link
Collaborator

sjudd commented Feb 8, 2018

GlideBuilder.build shouldn't be public and you shouldn't use it. There's no supported way to initialize Glide outside of modules.

@sjudd sjudd added the question label Feb 8, 2018
sjudd added a commit to sjudd/glide that referenced this issue Feb 8, 2018
It shouldn’t have been made visible and can’t 
safely be used directly outside of the library.

Fixes bumptech#2885
@wdkcjnwdwkldn
Copy link
Author

wdkcjnwdwkldn commented Feb 8, 2018

Why not?
Annotation processing generates excess code and spends build time. It's a bad way for android library.

Ok. I'll use deprecated init method.
Thanks.

@sjudd
Copy link
Collaborator

sjudd commented Feb 8, 2018

This has been covered before in a few ways, but here's a summary:

  1. It's not possible to safely initialize Glide outside of the library unless you do so eagerly in something like the Application object (and even then that's not entirely safe), which wastes resources.
  2. Given that initializing has to be done inside Glide to be safe, there's a limited set of mechanisms by which Glide is able to discover dependencies, including manifest parsing (slow and error prone) and annotation processing.
  3. The class overhead is small compared the size of the library as a whole.

Most of the overhead that annotation processing adds is at build time and that should be relatively easy to solve by just moving your GlideModule into a submodule in your project.

If you have any specific information you'd like to share about the overhead you think this process adds, please do consider providing it. There's always room for improvement.

@sjudd sjudd closed this as completed in 914060e Feb 9, 2018
@Tolriq
Copy link
Contributor

Tolriq commented Feb 9, 2018

@sjudd just saw this and currently I have not found a way to pass parameters to the module to for example pass custom cache directory, custom logger instance or things like that.

This makes moving to a sub module very complex, meaning annotation processor on main app, meaning no incremental build, meaning tons of time lost :)

Having a way to pass arguments or a way to disable annotation processor would be really nice improvement.

sjudd added a commit to sjudd/glide that referenced this issue Mar 6, 2018
-------------
Include the debug aar in release artifacts for Android projects.

We removed the release variant a while ago to speed up the build, which
has the side affect of removing the release aar from artifacts. Since
we expect the debug and release variants to be identical (hence why
we disabled the release variant), it should be safe to just use the
debug aar instead. We will have to specify it explicitly since android’s
rules unsurprisingly only add the release variant by default.

-------------
Bump version to 4.6.0

-------------
Update readme to 4.6.0

Also removes the old v4 dependency from maven deps, I don’t think it’s
necessary.

-------------
Change update_javadocs to use debugJavadocJar instead of release.

We’ve disabled the release variant.

-------------
Bump version to 4.7.0-SNAPSHOT

-------------
Add POM dependencies explicitly.

Fixes bumptech#2863.

-------------
Bump version to 4.6.1

-------------
Update readme to 4.6.1

-------------
Fix param mistake (bumptech#2873)

-------------
Update SimpleTarget javadoc to match v4 API.

-------------
Add javadoc for RequestOptions.apply/RequestBuilder.apply.

Related to bumptech#2858.

-------------
Add support for Uri data uris.

Previously we only supported data uris if they were provided as Strings.

Fixes bumptech#556.

-------------
Make GlideBuilder.build package private.

It shouldn’t have been made visible and can’t
safely be used directly outside of the library.

Fixes bumptech#2885

-------------
Handle notifications in MultiFetcher after cancellation.

We can’t guarantee that every fetcher implementation will strictly avoid
notifying after they’ve been cancelled.

This also improves the behavior in VolleyStreamFetcher so that it attempts to avoid notifying after cancel, although it still doesn’t make
any strict guarantee.

Fixes bumptech#2879

-------------
Re-enable -Werror for java compilation.

Related to bumptech#2886.

-------------
Fix a deprecation warning in DataUriTest.

-------------
gradle 4.5.1

-------------
deprecate fragments

-------------
add @deprecated to javadoc and suppress deprecations in code

-------------
Created by MOE: https://github.com/google/moe

MOE_MIGRATED_REVID=99229725401d5777e059da7b6331134bf73fbcdf

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185535564
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants