Keith Whitwell wrote on 2010-03-15 15:19: > On Mon, 2010-03-15 at 07:08 -0700, michal wrote: > >> michal wrote on 2010-03-12 15:00: >> >>> michal wrote on 2010-03-11 17:59: >>> >>> >>>> Keith Whitwell wrote on 2010-03-11 16:16: >>>> >>>> >>>> >>>>> On Thu, 2010-03-11 at 06:05 -0800, michal wrote: >>>>> >>>>> >>>>> >>>>> >>>>>> Keith Whitwell wrote on 2010-03-11 14:21: >>>>>> >>>>>> >>>>>> >>>>>> >>>>>>> On Thu, 2010-03-11 at 03:16 -0800, michal wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I would like to merge the branch in subject this week. This feature >>>>>>>> branch allows state trackers to bind sampler views instead of textures >>>>>>>> to shader stages. >>>>>>>> >>>>>>>> A sampler view object holds a reference to a texture and also >>>>>>>> overrides >>>>>>>> internal texture format (resource casting) and specifies RGBA swizzle >>>>>>>> (needed for GL_EXT_texture_swizzle extension). >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> Michal, >>>>>>> >>>>>>> I've got some issues with the way the sampler views are being generated >>>>>>> and used inside the CSO module. >>>>>>> >>>>>>> The point of a sampler view is that it gives the driver an opportunity >>>>>>> to do expensive operations required for special sampling modes (which >>>>>>> may include copying surface data if hardware is deficient in some way). >>>>>>> >>>>>>> This approach works if a sampler view is created once, then used >>>>>>> multiple times before being deleted. >>>>>>> >>>>>>> Unfortunately, it seems the changes to support this in the CSO module >>>>>>> provide only a single-shot usage model. Sampler views are created in >>>>>>> cso_set_XXX_sampler_textures, bound to the context, and then >>>>>>> dereferenced/destroyed on the next bind. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> The reason CSO code looks like this is because it was meant to be an >>>>>> itermediate step towards migration to sampler view model. Fully >>>>>> converting all existing state trackers is non-trivial and thus I chose >>>>>> this conservative approach. State trackers that do not care about extra >>>>>> features a sampler view provides will keep using this one-shot CSO >>>>>> interface with the hope that creation of sampler objects is lighweight >>>>>> (format matches texture format, swizzle matches native texel layout, >>>>>> etc.). >>>>>> >>>>>> >>>>>> >>>>>> >>>>> On the surface, this hope isn't likely to be fulfilled - lots of >>>>> hardware doesn't support non-zero first_level. Most cases of drivers >>>>> implementing sampler views internally are to catch this issue. >>>>> >>>>> Of course, it seems like your branch so leaves the existing >>>>> driver-specific sampler view code in place, so that there are >>>>> potentially two implementations of sampler views in those drivers. >>>>> >>>>> I guess this means that you can get away with the current implementation >>>>> for now, but it prevents drivers actually taking advantage of the fact >>>>> that these entities exist in the interface -- they will continue to have >>>>> to duplicate the concept internally until the state trackers and/or CSO >>>>> module start caching views. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>>> Ideally, everybody moves on and we stop using CSO for sampler >>>>>> views. I prefer putting my effort into incremental migration of state >>>>>> trackers rather than caching something that by definition doesn't need >>>>>> to be cached. >>>>>> >>>>>> >>>>>> >>>>>> >>>>> The CSO module exists to manage this type of caching on behalf of state >>>>> trackers. I would have thought that this was a sensible extension of >>>>> the existing purpose of the CSO module. >>>>> >>>>> Won't all state-trackers implementing APIs which don't expose sampler >>>>> views to the application require essentially the same caching logic, as >>>>> is the case with regular state? Wouldn't it be least effort to do that >>>>> caching once only in the CSO module? >>>>> >>>>> >>>>> >>>>> >>>> OK, I see your point. I will make the necessary changes and ping you >>>> when that's done. >>>> >>>> >>>> >>>> >>> Keith, >>> >>> I changed my mind, went ahead and implemented sampler view caching in >>> mesa state tracker, rather than inside cso context. >>> >>> I strongly believe that doing caching on cso side would be slower and >>> more complicated. A state tracker has a better understanding of the >>> relationship between a texture and sampler view. In case of mesa, this >>> is trivial 1-to-1 mapping. Later, when we'll need more sampler views per >>> texture, we can have a per-texture cache for that, and yes, the code for >>> that would be in cso. >>> >>> There are two other state trackers that need to be fixed: xorg and vega. >>> The transition should be similar to mesa -- I can help with doing that, >>> but I can't do it myself. Once that's done we can purge one-shot sampler >>> view wrappers. >>> >>> What do you think? >>> >>> >>> >> Keith, >> >> I just finished transforming mesa and auxiliary modules to new sampler >> view interfaces. The remaining bits are vega and xorg state trackers -- >> I will need help with them, but they could be fixed after the merge, as >> they are not broken, and just set sampler view in suboptimal fashion. >> >> Please review, thanks. >> > > > Michal, > > Did you get a chance to look at the double-refcounting (views + > textures) in the cso module? > > > They will go away once all the remaining consumers of cso_set/save/restore_sampler_textures() switch to cso_*_fragment_sampler_views().
------------------------------------------------------------------------------ Download Intel® Parallel Studio Eval Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta. http://p.sf.net/sfu/intel-sw-dev _______________________________________________ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
