Status Update
Comments
yb...@google.com <yb...@google.com>
da...@gmail.com <da...@gmail.com> #2
Note for dev team to check: If this isn't an error, CameraX should not log.
yb...@google.com <yb...@google.com> #3
Hello,
Thanks for filing this issue, this issue can be safely ignored - we have a todo for removing unnecessarily logging. This should be fixed shortly.
If there are other issues, please let us know.
yb...@google.com <yb...@google.com> #5
Branch: androidx-master-dev
commit 404aab04fc5ede6314fa93ff7b62431f9646e0e9
Author: Trevor McGuire <trevormcguire@google.com>
Date: Mon Apr 20 15:49:47 2020
Switch "Failed to set already detached use case online" to debug log
This can occur if the use case is detached before the camera's thread
has a chance to add the use case to the capture session. This should
not affect normal operation, so this error log message can be changed
to a debug log.
Relnote: "Converted error log related to detached use case to a debug log on Camera2CameraImpl."
Test: ./gradlew bOS
Bug: 154422490
Change-Id: I1a565e55af6e63792287221f8cbd9e7747fd6875
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2CameraImpl.java
da...@gmail.com <da...@gmail.com> #6
Can this log error message also be safely ignored?
Failed to set already detached use case online
ap...@google.com <ap...@google.com> #7
Sorry, I meant this one:
Failed to set already detached use case active
yb...@google.com <yb...@google.com> #8
Yes, it can be safely ignored. I will switch it to a debug log as well.
da...@gmail.com <da...@gmail.com> #9
Branch: androidx-master-dev
commit 0cff18a13097a05812b60c8bfb7968baf79464b4
Author: Trevor McGuire <trevormcguire@google.com>
Date: Mon Apr 27 11:52:06 2020
Switch "Failed to set already detached use case active" to debug log
This can occur if the use case is detached before the camera's thread
has a chance react to the use case being marked active. This should
not affect normal operation, so this error log message can be changed
to a debug log.
Bug: 154422490
Test: ./gradlew bOS
Change-Id: I2a99128be65c03af955753c230cb798789f8769f
M camera/camera-camera2/src/main/java/androidx/camera/camera2/internal/Camera2CameraImpl.java
yb...@google.com <yb...@google.com> #10
hmm, i see. We would need a new API for that but it might feel very weird (ViewHolder.getBindingAdapterViewType()) :/
Does NO_ISOLATION not work for your use case?
da...@gmail.com <da...@gmail.com> #11
I could use NO_ISOLATION or find another way to handle view types, maybe. I just wanted to do as less changes to the paging and recycler view classes and APIs as possible. Thank you for original fix, though. It would help a bit.
Also about ViewHolder.getBindingAdapterViewType(), yeah it sounds a bit off, but maybe Concat adapter should have the method like getBindingAdapterViewType(globalViewType). I haven't dig much into Concat adapter API, but I saw that you have some kind of map between local and global types and I suppose ViewTypeLookup could convert types for both strategies (Isolation and Shared) (ViewTypeStorage in API). Also ViewTypeStorage have the method like
NestedAdapterWrapper getWrapperForGlobalType(int globalViewType)
So seems like you can have adapter wrapper without using a position, like in getItemViewType(int position) in the original RV API. Maybe you can add open method to get local view type by using global.
The ConcatAdapterController already uses similar feature when it creates view holder (or triggers VH creation)
public ViewHolder onCreateViewHolder(ViewGroup parent, int globalViewType) {
NestedAdapterWrapper wrapper = mViewTypeStorage.getWrapperForGlobalType(globalViewType);
return wrapper.onCreateViewHolder(parent, globalViewType);
}
I may read the code wrong, but at a glimpse there is could be possibility to get local view type without using position, just global view type which I can get from VH.
yb...@google.com <yb...@google.com> #12
yea that could be as well. I've been trying to set up concat adapter to be as seemless as possible to the nested adapter but it does hit these cases where an explicit API becomes necessary :/.
If we were to provide this API in concat adapter, then the container adapter still needs to know that it is part of a concat adapter (and a reference to it) so I'm not sure how useful it will be (compared to also keeping a map or field in the VH yourself). Which is why i like the NO_ISOLATION solution better (and if you use layout ids as item types, that is usually good enough unless you have different VHs with the same layout)
But i guess you cannot use no isolation w/ the paging adapter :/ (though paging adapter is actually a very simple wrapper:
Description
Component used: ConcatAdapter
Version used: 1.2.0
Devices/Android versions reproduced on: any
The case is about two methods onViewRecycled and onFailedToRecycleView in ConcatAdapter. If you see the documentation of the base recycler view adapter it said that I can get a view holder position before it will be recycled by using ViewHolder#getBindingAdapterPosition() inside those two methods. But now comes ConcatAdapter who intercept those calls and execute its own code before it re-route them to nested adapters. The concat adapter executes the line which is removes viewHolder from mBinderLookup.
Concat Adapter triggers ConcatAdapterController.onViewRecycled:
The problem then comes out in nested adapter when I'm trying to get a view type of the view holder which is about to recycle:
I use holder.getBindingAdapterPosition() and receive NO_POSITION, of course. Because the Concat Adapter throws my call through ConcatAdapterController again by triggering getLocalAdapterPosition:
This method using mBinderLookup to search for the NestedAdapterWrapper for this view holder (mBinderLookup.get(viewHolder)), but the thing is that it is already gone, it was removed a moment ago in ConcatAdapterController.onViewRecycled. So now I do not see a possibility to get local view type using just a view holder in my nested adapter onViewRecycled method. And I can't use global view type from the view holder itself, because I'm using isolateViewTypes = true.
Even though the current implementation does not match the documentation of recycler view, I don't see how to use multiple view types for the nested adapter in isolation mode.